Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=891952 --- Comment #17 from Michael Schwendt <mschwendt@xxxxxxxxx> --- It's good that the FPC has answered like that. [...] Here's something unrelated to the reviewing guidelines. Not a blocker, just a recommendation: > # Modifiy Makefile.PL > sed … Makefile.PL > sed … Makefile.PL > sed … > # Remove ExtUtils::ParseXS tests > rm … Among many (most?) programmers, comments of that sort are considered superfluous. Worthless. And creating RPM Spec files is similar to programming scripts. Obviously, "sed" commands run on a Makefile.PL "modify" the file, so the comment doesn't need to point that out. Similarly for the "rm" command that removes several files. Much more interesting, and possibly even important, would be to explain _why_ that is being done? Why are the makefiles modified? Why is it necessary? And _what_ is the goal of those commands? Similarly for the "rm" command. _Why_ are the tests deleted? (especially if the package includes ExtUtils/ParseXS*) As the author of the spec file, you may be intimately familiar with what it does and why it does that. Perhaps you don't need any helpful comments in the spec file yourself, perhaps you would still remember even after a year what the commands do and why they do it. Nevertheless, I suggest replacing those comments with a more helpful rationale. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=qASC8v2wby&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review