https://bugzilla.redhat.com/show_bug.cgi?id=1181118 --- Comment #3 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> --- (In reply to Mattias Ellert from comment #2) > (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. Yeah, if you're using the same spec file for old EPEL, then it is better to leave things as they are. > > 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? Let's say that a person going through yumex and I see this package. My problem with current %description is that unless you already know what Globus Toolkit is, this description is not enough to get a rough overview. "grid" is too generic. It makes me think of electric power grid and math paper. If I look at http://www.globus.org/ I only know that this sofware has something to do with transferring or sharing data, but nothing precise. There are only buzzwords on that page. > 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 Yep, this is a very clear description. > > 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. That's why I wrote "consider". EPEL7 has %license, but earlier ones don't. https://fedorahosted.org/fpc/ticket/411#comment:9 provides a workaround to define %license on older EPELs. But since this landed in the guidelines, %license is now mandatory for Fedora. > > 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. OK. > > 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. OK. > > 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". Nevermind, evidently I got it reversed. http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Documentation_SubPackages indeed says "doc". > 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