[Bug 677043] Review Request: deheader - tool to find unnecesary includes in C/C++ source files

[bugzilla at redhat.com]

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=677043

--- Comment #5 from Michael Schwendt <mschwendt at gmail.com> 2011-04-01 09:32:26 EDT ---
> I don't see area for improvement in this small specfile any more ;)

I do. ;)


$ rpm -qpR deheader-0.6-1.fc14.noarch.rpm | grep -v ^rpm
/usr/bin/env  
python2  

$ head -1 /usr/bin/deheader 
#!/usr/bin/env python

This dependency ought to be improved. You've set a dependency on "python2", but
the script is executed with the first "python" found in $PATH. Changing the
shebang to /usr/bin/python (or /usr/bin/python2 if absolutely necessary) would
be the preferred way.


> Requires:       python2

$ repoquery --whatprovides python2
python-0:2.7-8.fc14.1.i686

It would be good practice to add a comment to such explicit "Requires". To
explain *why* an explicit dependency is necessary. Now I don't think this
dependency is necessary. But if it were, it would be added value to also add
"BuildRequires: python2", so you could check for the needed run-time dependency
already at build-time.


> Summary:        Find (optionally remove) unneeded includes in C or C++ source files

If one removes the brackets, an "and" is missing, so IMO better would be:
Find (and optionally remove) unneeded includes in C or C++ source files


> I think word is properly used in context ("many cohesions").

No big issue at all.  That plural form is very unsual/rare, even if Eric's
included "control" file also uses it. The plural for cohesion as a type of
measurement is "cohesion" as in "much cohesion" -- no cohesion, low/weak
cohesion, high/strong cohesion - it's an uncountable noun, isn't it: "25
cohesions?" when the level of cohesion is important. Probably one can derive
"cohesions" when referring to "cohesion in many files". Argh :-)

[...]

I notice this is your only package review request. Have you done any reviews
yet?

-- 
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.