Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: csync2 - A cluster synchronization tool https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=223633 wolfy@xxxxxxxxxxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |wolfy@xxxxxxxxxxxxxxxxxx OtherBugsDependingO|163776 |163778 nThis| | ------- Additional Comments From wolfy@xxxxxxxxxxxxxxxxxx 2007-01-23 21:13 EST ------- Good: - rpmlint checks do not return anything either on the source or on the binary rpm - package meets naming guidelines - package meets packaging guidelines - buildroot has the recommended value - license ( GPL ) is OK, text in %doc, matches source - spec file legible, in am. english - source is last version and matches upstream, sha1sum 8d94eb0c7ff997598be7b1cfc444bb74eae6712d csync2-1.33.tar.gz - package builds in mock for devel (i386) and FC6 (x86_64) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates, does not own foreign files or directories - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - available documentation is included, but small so no need for -docs - nothing in %doc affects runtime - no need for .desktop file - no static, headers, libtool or .pc files SHOULD - builds fine in mock (FC6/x86_64, devel/i386) - runs OK on FC6/x86_64 (once the proper instructions -- included in the rpm -- are followed, that is csync2 is added to /etc/services) Suggestion: the changelog entry should describe the actual modifications (especially since they are small - a one liner should suffice) rather then point to a bugzilla entry (it makes life simpler for someone examining the rpms, avoiding the need for an extra web query; not to mention that bugzilla might be down or simply not accessible) As a sidenote, you could have used a 1 line sed command in %install in order to modify the offending line in xinetd.conf, rather then creating a separate patch, but your method is perfectly fine. In this moment I see no blockers, but before approving this package I would like for someone more experienced to comment on the following: In order to start the program (via xinetd), /etc/services must be modified, adding a line specifying the port and protocol used by csync2 and SSL certificates must be generated for each box where the program will run Editing /etc/services could be handled either in a %post entry (in the sources there is a spec file which does the addition, but does not verify if the entry already exists and neither removes it at uninstall time), or (as the packager has chosen) manually by the admin. My question is: which one of this two methods is the proper procedure for Fedora ? -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review