https://bugzilla.redhat.com/show_bug.cgi?id=1181118 --- Comment #2 from Mattias Ellert <mattias.ellert@xxxxxxxxxxxx> --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #1) Many thanks for taking on the review! > 1. remove pkgdocdir definition, Fedora versions which didn't have this are > EOL > 2. remove BuildRoot > 3. remove rm -rf %{buildroot} > 4. remove %clean This package, like the already existing 50+ Globus Toolkit packages are intended for EPEL as well. My impression is that they are in fact more used in EPEL than in Fedora. > 5. %description reads like an advertisement. It should at least mention what > "grid" is. Who develops it is not crucial. Also the last paragraph "The > %{name package contains..." is not really useful, since it repeats the > Summary. The first part of the description is taken from upstream's website http://toolkit.globus.org/toolkit/. It is upstream's own description of the software, so I think it is relevant. The same sentence appears in the description of the 50+ Globus Toolkit packages that already exists in Fedora. I can recognize that "grid" is not the same hype and buzz word it was 5-10 years ago, but the meaning of the word hasn't changed. Is it really forgotten to the extent that it is not possible to use it anymore? For most packages the short description is descriptive enough, but I guess the meaning of "Network Manager" is not self-explanatory and its meaning is overloaded. I can add the introduction from the Doxygen documentation to the description: The Globus Net Manager library is a plug-in point for network management tasks, such as: - selectively open ports in a firewall and allow these ports to be closed when transfers are complete - configure a virtual circuit based on a site policy and route traffic over this circuit - route network traffic related to a task over a particular network > 7. consider using %license for GLOBUS_LICENSE > (and add %global _docdir_fmt %{name} to avoid having multiple copies) You are a very fast adopter of new guidelines - timewarp almost. The guidelines didn't mention %license at all before January 15 - the day after you wrote your comment :-). Using %license would require using %if %else %endif conditionals to do the right thing for EPEL 5 and 6. It is not very difficult to do if you really insist on it. I will probably have to do it eventually. > 8. %files should have (note extra dot) > %{_libdir}/libglobus_net_manager.so.* Thank you for pointing this one out - my mistake. > 9. where does Source8 come from? I am not sure what you are asking here. There is no upstream source for this file anywhere. It is a collection of links to the upstream online documentation compiled for the purpose of creating the package. Since the upstream project groups their packages and provide documentation per group, all packages in the same group have the same README file, so in this case it was copied from an other package in the XIO group. These files were added to the packages in order to address complaints about missing documentation. > 10. Consider using %make_install instead of the open-coded version. Why? It is too close to %makeinstall which is not the right thing to do. I would confuse myself by using it and I would have to run manual expansions of the macro every time I modified the package to check that I was still using the right one. Also, it is not available on EPEL 5 and would need to be conditionally defined in the specfile. > 11. -docs is preferred to -doc for the package name. I do not believe there is a consensus about that. Statistics querying the repo (F21) gives 133 packages named -docs and 3602 packages named -doc (or 1193 if the texlive packages are excluded). Even without the texlive packages there is a big majority for -doc over -docs. I also prefer consistency - all existing Globus Toolkit packages that provide a documentation package uses the -doc naming. Suddenly using something else for a new package would be terribly confusing. For me personally -docs gives the wrong association since I read it as "documents" and not the intended "documentation". Updated version: Spec URL: http://www.grid.tsl.uu.se/review/globus-net-manager.spec SRPM URL: http://www.grid.tsl.uu.se/review/globus-net-manager-0.6-1.fc21.src.rpm -- 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