Red Hat Bugzilla – Bug 219610
Review Request: uncrustify - code beautifier
Last modified: 2007-11-30 17:11:51 EST
Spec URL: http://nbecker.dyndns.org:8080/RPM/uncrustify.spec
SRPM URL: http://nbecker.dyndns.org:8080/RPM/uncrustify-0.30-1.fc6.src.rpm
Description: Source Code Beautifier for C, C++, C#, D, Java, and Pawn
Hi Neil, here are some observations and I'll post a more thorough review
as soon as I get a chance:
+ source matches upstream
+ builds in mock on FC6 i386
- rpmlint reports:
W: uncrustify summary-not-capitalized reformat source
W: uncrustify invalid-license gpl
I just downloaded, build and rpmlinted this. I get none of the above issues.
Neal, have you made changes to the available files since 12/14, or am I missing
Yes, I updated the files. I'm sorry for any confusion.
Hi Neal, when you update the files *please* always increase the EVR and
please post a comment with a link to the new SRPM. I find it quite
confusing when people skip those two steps.
I'm very busy today/tomorrow but will make an effort to finish the review
That explains quite a bit. :) I would have envisioned a bumped release number
and new URLs.
Would it not be helpful to include a copy of the default config
(http://uncrustify.sourceforge.net/default.cfg) as ~/.uncrustify.cfg? Running
it right after installation results in a complaint, and you can't even
uncrustify --show-config -o ~/.uncrustify.cfg without it already existing.
If I manually create ~/.uncrustify.cfg from default.cfg, it works fine.
I agree that's a problem. (I didn't write this code :))
Anyway, how would that recommendation work? (I don't think it would).
Here is my proposal, which I think is consistent with other Fedora apps. On
install, a message is printed telling the user they need to do this. OK?
After thinking for two seconds, I realized my above comment won't work. I think
including default.cfg as /etc/uncrustify.cfg, and having it check that first,
then override with ~/uncrustify.cfg, would be better.
That is really how it should work, but I didn't write it. If nobody has
strong objections, I think I'd rather just print the message then spend time
figuring out how to do fix the code.
I've played around with it, and am close, but I couldn't get it to compile. Not
worth further time.
$ sha1sum uncrustify-0.30-1.fc6.src.rpm
+ builds in mock for FC6 i386
+ rpmlint is silent
+ license is OK and correctly included
+ spec is very simple and clean
+ no shared libs and no *.la
+ perms and dir ownership OK
+ %clean OK
could be improved:
- the "Summary: Reformat source" line is a little ambiguous and it
might be more helpful to say "Tool for source code reformatting"
or similar -- but that's just a suggestion, not a blocker
Its unfortunate how the config files are handled but I agree that it
is an upstream problem.
Changed summary for tracking purposes.
Hi Neal, I don't mean to be picky but packages that are successfully
built and pushed:
should (please) be closed as NEXTRELEASE (not UPSTREAM). Thanks!
Yes, that was unintentional.
oh well... :-)