8:30 a.m.
----- Original Message -----
From: "Eyal Edri" <eedri(a)redhat.com>
To: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Friday, January 4, 2013 5:39:52 PM
Subject: Re: [Engine-devel] Java code formatting
----- Original Message -----
> From: "David Jaša" <djasa(a)redhat.com>
> To: "engine-devel" <engine-devel(a)ovirt.org>
> Sent: Friday, January 4, 2013 4:37:13 PM
> Subject: Re: [Engine-devel] Java code formatting
>
> Doron Fediuck píše v St 02. 01. 2013 v 03:24 -0500:
> >
> > ----- Original Message -----
> > > From: "Livnat Peer" <lpeer(a)redhat.com>
> > > To: "Doron Fediuck" <dfediuck(a)redhat.com>
> > > Cc: "engine-devel" <engine-devel(a)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(a)redhat.com>
> > > >> To: "Doron Fediuck" <dfediuck(a)redhat.com>
> > > >> Cc: "engine-devel" <engine-devel(a)ovirt.org>
> > > >> Sent: Tuesday, January 1, 2013 5:05:09 PM
> > > >> Subject: Re: [Engine-devel] Java code formatting
> > > >>
> > > >>
> > > >>
> > > >> ----- Original Message -----
> > > >>> From: "Doron Fediuck" <dfediuck(a)redhat.com>
> > > >>> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> > > >>> Cc: "engine-devel" <engine-devel(a)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(a)redhat.com>
> > > >>>> To: "Doron Fediuck"
<dfediuck(a)redhat.com>
> > > >>>> Cc: "engine-devel"
<engine-devel(a)ovirt.org>
> > > >>>> Sent: Tuesday, January 1, 2013 4:53:49 PM
> > > >>>> Subject: Re: [Engine-devel] Java code formatting
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>> ----- Original Message -----
> > > >>>>> From: "Doron Fediuck"
<dfediuck(a)redhat.com>
> > > >>>>> To: "Alon Bar-Lev"
<alonbl(a)redhat.com>
> > > >>>>> Cc: "engine-devel"
<engine-devel(a)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(a)redhat.com>
> > > >>>>>> To: "Doron Fediuck"
<dfediuck(a)redhat.com>
> > > >>>>>> Cc: "engine-devel"
<engine-devel(a)ovirt.org>
> > > >>>>>> Sent: Tuesday, January 1, 2013 4:33:20 PM
> > > >>>>>> Subject: Re: [Engine-devel] Java code formatting
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>> ----- Original Message -----
> > > >>>>>>> From: "Doron Fediuck"
<dfediuck(a)redhat.com>
> > > >>>>>>> To: "Alon Bar-Lev"
<alonbl(a)redhat.com>
> > > >>>>>>> Cc: "engine-devel"
<engine-devel(a)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(a)redhat.com>
> > > >>>>>>>> To: "Doron Fediuck"
<dfediuck(a)redhat.com>
> > > >>>>>>>> Cc: "engine-devel"
<engine-devel(a)ovirt.org>
> > > >>>>>>>> Sent: Tuesday, January 1, 2013 4:17:18
PM
> > > >>>>>>>> Subject: Re: [Engine-devel] Java code
formatting
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>> ----- Original Message -----
> > > >>>>>>>>> From: "Doron Fediuck"
<dfediuck(a)redhat.com>
> > > >>>>>>>>> To: "engine-devel"
<engine-devel(a)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....
> > > >
> > > > 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.
if the jenkins jobs will run per gerrit patch and not per commit,
(it's possible to run it via jenkins, like run other jobs per
patch).
then it's not too late, and less messy than defining specific hooks
for it.
Eyal.
Right, we actually had something similar in the past
while .NET was still around...
>
> 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(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
>
> --
>
> David Jaša, RHCE
>