[Bug 1094013] Review Request: vim-ledger - Vim plugin for use with Ledger, the double-entry accounting system

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1094013



--- Comment #3 from Jamie Nguyen <jamielinux@xxxxxxxxxxxxxxxxx> ---
(In reply to NIWA Hideyuki from comment #1)
> Hi 
> This is my informal review. I comment about the spec file etc.

Thanks! :)


> 1. %global commit     38d7a037
> 
> Referring to the following.
> https://fedoraproject.org/wiki/Packaging:SourceURL#Github

Fixed.


> 2. Group:             Applications/Editors
> 
> Remove them, no need to keep them now.

They are optional for Fedora, but kept for EPEL:
https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag


> 3.
> URL is necessary for the Source tag. 
> https://fedoraproject.org/wiki/Packaging:SourceURL#Github
> 
> Please remove source10 and source20.

Fixed.


> 4. Requires:          vim-common
> Requires(post):    vim
> Requires(postun):  vim
> 
> Please add comments on explicit dependencies. 

Added.


> 5. %description
> %{summary}.
> 
> Please write a detailed content in description. 

Done.


> 6. %prep
> %setup -q -n %{name}-%{checkout}
> cp -p %{SOURCE20} .
> 
> Why is "cp -p %{SOURCE20}" necessary here?

Removed.


> 7. %postun
> rm %{installdir}/doc/tags
> 
> Please remove "rm %{installdir}/doc/tags".
> It is dangerous rm in %postun.

Removed.


Spec URL: http://jamielinux.fedorapeople.org/ledger/vim-ledger.spec
SRPM URL:
http://jamielinux.fedorapeople.org/ledger/SRPMS/vim-ledger-0-0.2.20140504git06c6873.fc21.src.rpm

* Mon Jul 14 2014 Jamie Nguyen <jamielinux@xxxxxxxxxxxxxxxxx> -
0-0.2.20140504git06c6873f
- update to latest commit
- use correct GitHub URL
- fix explicit Requires
- comments for explicit Requires
- amend scriptlet commands
- extend %%description

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]