Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at

Bug 219610

Summary: Review Request: uncrustify - code beautifier
Product: [Fedora] Fedora Reporter: Neal Becker <ndbecker2>
Component: Package ReviewAssignee: Ed Hill <ed>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: gwync, panemade
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-01-03 19:25:29 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 163779    

Description Neal Becker 2006-12-14 13:55:53 UTC
Spec URL:
Description: Source Code Beautifier for C, C++, C#, D, Java, and Pawn

Comment 1 Ed Hill 2006-12-14 22:23:20 UTC
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

Comment 2 Gwyn Ciesla 2006-12-21 13:28:08 UTC
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

Comment 3 Neal Becker 2006-12-21 13:33:15 UTC
Yes, I updated the files.  I'm sorry for any confusion.

Comment 4 Ed Hill 2006-12-21 13:47:48 UTC
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 
on Saturday.

Comment 5 Gwyn Ciesla 2006-12-21 13:54:50 UTC
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
( 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.

Comment 6 Neal Becker 2006-12-21 13:59:55 UTC
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?

Comment 7 Gwyn Ciesla 2006-12-21 14:01:11 UTC
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.

Comment 8 Neal Becker 2006-12-21 14:04:40 UTC
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.

Comment 9 Gwyn Ciesla 2006-12-21 14:36:23 UTC
I've played around with it, and am close, but I couldn't get it to compile.  Not
worth further time.

Comment 10 Ed Hill 2006-12-26 04:19:52 UTC
$ sha1sum uncrustify-0.30-1.fc6.src.rpm  
d4a0326889cd994e29422723fd9be1517e41edc8  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.


Comment 11 Christian Iseli 2007-01-03 00:45:40 UTC
Changed summary for tracking purposes.

Comment 12 Ed Hill 2007-01-04 03:06:24 UTC
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!

Comment 13 Neal Becker 2007-01-04 12:05:44 UTC
Yes, that was unintentional.

Comment 14 Christian Iseli 2007-01-04 12:24:52 UTC
oh well... :-)