Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=459444 Tom "spot" Callaway <tcallawa@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tcallawa@xxxxxxxxxx Flag|fedora-review+ |fedora-review? --- Comment #16 from Tom "spot" Callaway <tcallawa@xxxxxxxxxx> 2009-01-13 12:02:37 EDT --- (In reply to comment #6) > I made the change and re-uploaded the srpms. You can find them in the same > locations mentioned in comment #4 > > Thanks! > --Abhi Abhi, there are a few more changes I'd like to see you make, and then I'd be willing to sponsor you. Chris, please look over this, as you should have caught some of these before approving this package. * Please update this package to the latest release, which seems to be 1.0.68 * In Source0, use the full URL to the upstream source. RPM is smart enough to know how to parse that. (Don't worry, it doesn't try to download it, it uses the local file) * For Patch0, please submit it for upstream inclusion, and leave a comment next to it describing where you sent it (a URL to an open bug is sufficient, for example). See: https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment * The License tag is wrong, it should actually be: GPLv3+ * It doesn't look like the upstream source comes with a copy of the license. You should contact upstream to ask them to add it for future releases (and when they do, it should be packaged as %doc). See: https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text * Flesh out the Summary a bit more, so that it avoids acronyms. I have no idea what a TDB is. Even simply "A Clustered Database" would be better. * In your CFLAGS, you're overriding the Fedora optimization flag by passing -O0. Don't do that. :) * You're also using ./configure rather than the %configure macro. Please use %configure instead (it defines all of the flags you're passing in the same way). In your spec, you can simply do: CFLAGS="$(echo '%{optflags}') $EXTRA -D_GNU_SOURCE -DCTDB_VERS=\"%{version}-%{release}\"" %configure * Be sure you're using "%defattr(-,root,root,-)" in every %files section. * Also, be sure you're passing four parameters for %attr (if you don't need to change the fourth field <dir mode>, just use a - ). * Your %files sections need to be cleaned up. You're duplicating files there. One trick that should help you out quite a bit: If you specify a directory and end it with a /, RPM will own that directory, and all the files and directories below it. So, for example, instead of: %{_sysconfdir}/ctdb %{_sysconfdir}/ctdb/events.d/00.ctdb %{_sysconfdir}/ctdb/events.d/10.interface %{_sysconfdir}/ctdb/events.d/40.vsftpd ... Just use: %{_sysconfdir}/ctdb/ When you're test building your package, RPM will complain about these duplicated files. Be sure you've got rid of any such warnings. ######### If you have not done so, please take a moment and read: https://fedoraproject.org/wiki/Packaging/Guidelines https://fedoraproject.org/wiki/Packaging/ReviewGuidelines When you've gotten all of those items resolved, please upload a new SRPM and put a link in this bugzilla ticket. I'll come back and look at it and then re-approve and sponsor you. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review