https://bugzilla.redhat.com/show_bug.cgi?id=817271 --- Comment #17 from Alec Leamas <leamas.alec@xxxxxxxxx> --- (In reply to comment #16) > Ok, a couple of nit-picks: > > 1. The source tag could use the %name and %version tags a little more: > Source0: > http://nightly.openerp.com/6.1/nightly/src/openerp-%{version}%{oe_rel}.tar.gz > could be: > Source0: > http://nightly.openerp.com/%{version}/nightly/src/%{name}- > %{version}%{oe_rel}.tar.gz > > unless the 6.1 doesn't get the patch level version number when/if it occurs. I certainly could, and I will if you insist. However, using openerp instead of name is on purpose and as I understand it according to the guidelines. The macro section explicitly says that using macros (besides paths) is a question of personal preferences, and I prefer writing the name "in clear". > > 2. If your using "install" to install additional files from the source then > you need to add the -p option to preserve time stamps, i.e.: > > install -m 644 -D install/openerp-server.conf \ > %{buildroot}%{_sysconfdir}/openerp/openerp-server.conf > to: > install -pm 644 -D install/openerp-server.conf \ > %{buildroot}%{_sysconfdir}/openerp/openerp-server.conf > Fixed. "blushes" > 3. I'm not a reviewers that insists on using %dir in %files and calling out > each directory/file underneath it separately but if you're going to glob a > whole directory it's a good practice to leave a trailing slash on the entry > so it's obvious to someone else looking at your spec: > > %{_datadir}/openerp > to: > %{_datadir}/openerp/ Fixed > > 4. I assume some of the strange permissions are needed for functionality or > security? It would probably be a good idea to add comments to the entries in > %files where you're assigning non-standard perms. Fixed Updating in place, same links, no release bumb (which actually is OK, promise :) ) -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review