[Engine-devel] Java code formatting

[Doron Fediuck]

----- Original Message -----
> From: "Alon Bar-Lev" <alonbl at redhat.com>
> To: "Doron Fediuck" <dfediuck at redhat.com>
> Cc: "engine-devel" <engine-devel at ovirt.org>
> Sent: Tuesday, January 1, 2013 5:05:09 PM
> Subject: Re: [Engine-devel] Java code formatting
> 
> 
> 
> ----- Original Message -----
> > From: "Doron Fediuck" <dfediuck at redhat.com>
> > To: "Alon Bar-Lev" <alonbl at redhat.com>
> > Cc: "engine-devel" <engine-devel at ovirt.org>
> > Sent: Tuesday, January 1, 2013 5:00:20 PM
> > Subject: Re: [Engine-devel] Java code formatting
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Alon Bar-Lev" <alonbl at redhat.com>
> > > To: "Doron Fediuck" <dfediuck at redhat.com>
> > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > Sent: Tuesday, January 1, 2013 4:53:49 PM
> > > Subject: Re: [Engine-devel] Java code formatting
> > > 
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "Doron Fediuck" <dfediuck at redhat.com>
> > > > To: "Alon Bar-Lev" <alonbl at redhat.com>
> > > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > > Sent: Tuesday, January 1, 2013 4:48:22 PM
> > > > Subject: Re: [Engine-devel] Java code formatting
> > > > 
> > > > 
> > > > 
> > > > ----- Original Message -----
> > > > > From: "Alon Bar-Lev" <alonbl at redhat.com>
> > > > > To: "Doron Fediuck" <dfediuck at redhat.com>
> > > > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > > > Sent: Tuesday, January 1, 2013 4:33:20 PM
> > > > > Subject: Re: [Engine-devel] Java code formatting
> > > > > 
> > > > > 
> > > > > 
> > > > > ----- Original Message -----
> > > > > > From: "Doron Fediuck" <dfediuck at redhat.com>
> > > > > > To: "Alon Bar-Lev" <alonbl at redhat.com>
> > > > > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > > > > Sent: Tuesday, January 1, 2013 4:28:15 PM
> > > > > > Subject: Re: [Engine-devel] Java code formatting
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > ----- Original Message -----
> > > > > > > From: "Alon Bar-Lev" <alonbl at redhat.com>
> > > > > > > To: "Doron Fediuck" <dfediuck at redhat.com>
> > > > > > > Cc: "engine-devel" <engine-devel at ovirt.org>
> > > > > > > Sent: Tuesday, January 1, 2013 4:17:18 PM
> > > > > > > Subject: Re: [Engine-devel] Java code formatting
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > ----- Original Message -----
> > > > > > > > From: "Doron Fediuck" <dfediuck at redhat.com>
> > > > > > > > To: "engine-devel" <engine-devel at ovirt.org>
> > > > > > > > Sent: Tuesday, January 1, 2013 4:07:53 PM
> > > > > > > > Subject: [Engine-devel] Java code formatting
> > > > > > > > 
> > > > > > > > Hi,
> > > > > > > > Recently I saw many patches with multiple code
> > > > > > > > re-formatting.
> > > > > > > > When looking into it, we saw that many people didn't
> > > > > > > > use
> > > > > > > > the
> > > > > > > > project
> > > > > > > > policy, and now we have many files with bad formatting.
> > > > > > > > 
> > > > > > > > So I just posted a big ugly fix for this[1], and
> > > > > > > > hopefully
> > > > > > > > if
> > > > > > > > accepted
> > > > > > > > people should start using the right conventions and
> > > > > > > > reduce
> > > > > > > > the
> > > > > > > > amount
> > > > > > > > of non-relevant changes we see in the patches.
> > > > > > > > 
> > > > > > > > I'm aware of the fact that this may create some issues
> > > > > > > > when
> > > > > > > > porting
> > > > > > > > patches, but better sooner than later.
> > > > > > > > Doron.
> > > > > > > > 
> > > > > > > > [1] http://gerrit.ovirt.org/#/c/10541/1
> > > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > These automatic conversions are not better than current
> > > > > > > state,
> > > > > > > also
> > > > > > > I
> > > > > > > don't think that this is that important. If you want
> > > > > > > machine
> > > > > > > written
> > > > > > > code, then also provide commit hook to reformat anything,
> > > > > > > and
> > > > > > > probably machines to read it.
> > > > > > > 
> > > > > > > I, personally, think that this change over the sources I
> > > > > > > manage
> > > > > > > did
> > > > > > > not do any good.
> > > > > > > 
> > > > > > > Regards,
> > > > > > > Alon
> > > > > > 
> > > > > > Alon,
> > > > > > there's a formatting convention for the project set long
> > > > > > ago.
> > > > > > If you feel it needs to be fixed, go ahead and suggest a
> > > > > > fix
> > > > > > for
> > > > > > the xml.
> > > > > > Otherwise we end up in the current chaos, where every 2nd
> > > > > > or
> > > > > > 3rd
> > > > > > patch carries unneeded changes.
> > > > > > 
> > > > > 
> > > > > What do you mean unneeded changes? how do you prevent this in
> > > > > future?
> > > > > 
> > > > > Alon
> > > > 
> > > > Unneeded changes is when you get one line of code fixed due to
> > > > a
> > > > bug,
> > > > and many others re-indented.
> > > > Best prevention is if people would make sure to use the same
> > > > conventions.
> > > > We also have a checkstyle which monitors important issues such
> > > > as
> > > > trailing
> > > > white spaces, localization, etc.
> > > > 
> > > 
> > > But how do you prevent this in future, not all working in same
> > > editor
> > > nor same styles.
> > > You can say that once in a while you perform cross over auto
> > > re-format...
> > > I am not native Java programmer but this sounds very strange
> > > thing
> > > to
> > > do, I don't think I know of any project that does that.
> > > 
> > > The main problem is that people commit stuff they don't touch due
> > > to
> > > their editor behavior, which tread the whole source as if it was
> > > at
> > > its disposal, while cation should be taken not to modify extra
> > > stuff.
> > > 
> > > So maybe just to reject patches that touches lines which are not
> > > belong to the patch it-self.
> > > 
> > > Alon
> > 
> > I'm fine with rejecting patches with irrelevant changes.
> > Still sometimes you get new files and this repeats itself.
> > The whole point of a convention is that people keep it.
> > Most major IDEs today can use the same formatting XML we now have.
> > So it's really a matter of self-discipline.
> > 
> 
> OK, I suggest we start rejecting patches with irrelevant changes, and
> placing these irrelevant changes in their own patch to be relevant.
> 
> I don't think that we can enforce XML formatting for all editors out
> there... unless there is a cli utility to reformat before commit, or
> even do this at gerrit side, we should be somewhat flexible.
> 
> But these are only my two cents.
> 
> Alon

Thanks to Ofri we can now both automate it and monitor it using
http://maven-java-formatter-plugin.googlecode.com/svn/site/0.3.1/examples.html.

We should be able to add it to the root pom and make sure it won't
happen again.