For some reason Eclipse doesn't consider this to be a formatting operation
(at least I couldn't find it), but it considers it a "cleanup." I created
an ArgoEclipse cleanup profile and enabled it for all the projects.
Separately, I enabled some save actions for all the projects (thanks for the
suggestion Brian) including fixing conditionals and organizing imports, but
not running the full blown formatter. I'm a little leery of changes to the
code after the developer has stopped looking at it (ie during save) and
especially of the formatter which doesn't always make the best choices
subjectively.
We'll experiment with the save actions for a little while and see how folks
like them. If they're not working out, we can turn them back off again. In
the short term, they'll probably cause some perturbations in the order of
imports, but this should settle down quickly. By the way, the thinking
behind the order of the imports is most general to most specific, so they go
java->javax->org->com->org.eclipse->org.tigris (ie
GEF)->org.argouml->argoeclipse.
Tom
On Fri, Jul 18, 2008 at 1:48 PM, Brian Hudson <[EMAIL PROTECTED]>
wrote:
> Eclipse can do this in save actions. Perhaps we should enable project
> specific save actions and have Eclipse fix this, run the formatter, organize
> imports, etc... on save? This would ensure that an indent mistake like this
> would not get checked in (the formatter would fix it) and ensure that
> conditionals use blocks.
>
> Brian
>
>
> On Fri, Jul 18, 2008 at 1:40 PM, Tom Morris <[EMAIL PROTECTED]> wrote:
>
>> I wasted a whole bunch of hours recently tracking down what turned out to
>> be a bug in MDR that looked like this:
>>
>> if (item.endName.equals(assocStorable.getEnd2Name())) {
>> assoc.refRemoveLink((RefObject) temp, thisObject);
>>
>>
>> } else {
>> assoc.refRemoveLink(thisObject, (RefObject) temp);
>> if (item.isAggregate)
>> ((RefObject) temp).refDelete();
>> }
>>
>> where the visual appearance didn't match the actual logic. Checkstyle
>> probably would have flagged that the indentation didn't match the logic
>> level, but to be extra safe I've made Checkstyle stricter and it will now
>> require braces for all conditionals. Please observe this requirement for
>> new code and clean up any module that you are working on (no need to go back
>> and rework things that aren't going to be touched for some other reason).
>>
>> I used to think single line if statements were ok without braces, but they
>> just end up getting broken into two lines and indented and then someone adds
>> an else arm and it's all just a slippery slope, so we're just going to
>> require braces for everything. I generally prefer vertically compact code,
>> but, in this case, I think the extra white space is worth the additional
>> safety.
>>
>> Tom
>>
>
>
