Forum OpenACS Improvement Proposals (TIPs): Re: TIP #61: (Proposed) Guidelines for CVS committers

Collapse
Posted by Lars Pind on
Approved, except for this, which I'd like to have clarified:

"Code should only be reformatted when functionality is changed"

I think it's actually superior to have reformatting done separately from any other change, since then it's easier to figure out what changed functionally in the commmit.

I'd like to see some code formatting standards as well, like indents, style for declaring procs in namespaces, etc. This is something that makes reading code and fixing bugs a bit easier. And reformatting to follow the standards should be okay in my opinion.

Collapse
Posted by Jeff Davis on
"Code should only be reformatted when functionality is changed"

The intent is to avoid mixing reformatting with changes and by "functionality" the meaning was control flow so if you went from:

 
blah
bleh
foo
to
blah 
if {![zim]} { 
    bleh
    foo
}
you would indent bleh and foo properly.

I agree that you want to seperate commits to bring code into conformance with formatting conventions from actual code changes and it's something we should add as part of the guidelines. On the other hand, recognize that reformatting comes with quite a cost to people maintaining seperate archives for their sites, as it can mean tremendous numbers of conflicts.

I did a little checking to confirm what we already knew which was that tcl is overwhelmingly indented 4 spaces (something like 15:1 versus 2).

I think if we wanted to reformat code to bring it into compliance with standards (I think we need to be clearer about what those are) there are good times to it. Best near term would be when 5.1 is pretty much stable and 5.2 is not yet branched. That should be pretty soon (probably after the dotlrn bug bash would be a good time).

Collapse
Posted by Joel Aufrecht on
Marking approved