[Engine-devel] Java code formatting

David Jaša djasa at redhat.com
Fri Jan 4 14:37:13 UTC 2013 

Doron Fediuck píše v St 02. 01. 2013 v 03:24 -0500:
> 
> ----- Original Message -----
> > From: "Livnat Peer" <lpeer at redhat.com>
> > To: "Doron Fediuck" <dfediuck at redhat.com>
> > Cc: "engine-devel" <engine-devel at ovirt.org>
> > Sent: Tuesday, January 1, 2013 5:58:15 PM
> > Subject: Re: [Engine-devel] Java code formatting
> > 
> > On 01/01/2013 05:39 PM, Doron Fediuck wrote:
> > > 
> > > 
> > > ----- 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.
> > 
> > From a quick look this plugin looks like formatted and not
> > format-validator, if this is the case it is less useful as we can't
> > add
> > a jenkins job to monitor the expected formatting.
> > 
> > I think that we should clean the formatting (once again...) in our
> > code
> > base only after we'll have a valid way to enforce it, otherwise we
> > are
> > going to have the same thread every few months (we already had it
> > twice
> > or more) and we all hate this massive formatting patches that comes
> > after the thread...
> > 
> > Livnat
> > 
> 
> Livnat,
> it's easy enough to have a Jenkins job running this plugin.

Isn't Jenkins too late in the game for checks of incoming code? IIRC
there do exist pre-commit hooks for gerrit that run pylint/pyflakes on
python files, that should IMO be the very place to check Java coding
style, too.

David

> Then git status will tell you if something changed, and if so
> you email the changes to patch owner (and nack the patch). We
> had similar jobs in WPF era and it's quite simple to handle.
> So I see no reason why not to start it now. The sooner we start,
> less noise and unneeded code changes will be added.
> 
> better
> _______________________________________________
> Engine-devel mailing list
> Engine-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel

-- 

David Jaša, RHCE

SPICE QE based in Brno
GPG Key:     22C33E24 
Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24

More information about the Engine-devel mailing list