[Engine-devel] Proposal for new commit msg design for engine commits

Eyal Edri eedri at redhat.com
Sat Jul 20 17:34:28 UTC 2013 

OK, Rome wasn't built in a day.

To move things forward, 
I propose we'll just improve current commit header template to include more relevant code
areas [1], and start looking into mapping all code to the relevant components 
(either via renaming folders, adding a metadata file under each folder mapping the files/classnames/directory names or using automated tools like sonar)

[1] instead of               <core | restapi | tools | history | engine | userportal | webadmin>
    change to something like <core | restapi | tools | userportal | webadmin | network | storage | virt | packaging>

Eyal.

----- Original Message -----
> From: "Moran Goldboim" <mgoldboi at redhat.com>
> To: "Eyal Edri" <eedri at redhat.com>
> Cc: "Fabian Deutsch" <fabiand at redhat.com>, "engine-devel" <engine-devel at ovirt.org>, "infra" <infra at ovirt.org>
> Sent: Sunday, July 14, 2013 6:07:05 PM
> Subject: Re: [Engine-devel] Proposal for new commit msg design for engine commits
> 
> On 07/11/2013 11:57 AM, Eyal Edri wrote:
> >
> > ----- Original Message -----
> >> From: "Fabian Deutsch" <fabiand at redhat.com>
> >> To: "Eyal Edri" <eedri at redhat.com>
> >> Cc: "Alon Bar-Lev" <alonbl at redhat.com>, "engine-devel"
> >> <engine-devel at ovirt.org>, "infra" <infra at ovirt.org>
> >> Sent: Thursday, July 11, 2013 11:41:24 AM
> >> Subject: Re: [Engine-devel] Proposal for new commit msg design for engine
> >> commits
> >>
> >> Am Mittwoch, den 10.07.2013, 15:27 -0400 schrieb Eyal Edri:
> >>> ----- Original Message -----
> >>>> From: "Fabian Deutsch" <fabiand at redhat.com>
> >>>> To: "Alon Bar-Lev" <alonbl at redhat.com>
> >>>> Cc: "engine-devel" <engine-devel at ovirt.org>, "infra" <infra at ovirt.org>
> >>>> Sent: Tuesday, July 9, 2013 3:54:06 PM
> >>>> Subject: Re: [Engine-devel] Proposal for new commit msg design for
> >>>> engine
> >>>> commits
> >>>>
> >>>> Am Dienstag, den 09.07.2013, 08:49 -0400 schrieb Alon Bar-Lev:
> >>>>>
> >>>>> ----- Original Message -----
> >>>>>> From: "Yair Zaslavsky" <yzaslavs at redhat.com>
> >>>>>> To: "Alon Bar-Lev" <alonbl at redhat.com>
> >>>>>> Cc: "Eyal Edri" <eedri at redhat.com>, "engine-devel"
> >>>>> <engine-devel at ovirt.org>, "infra" <infra at ovirt.org>
> >>>>>> Sent: Tuesday, July 9, 2013 3:42:24 PM
> >>>>>> Subject: Re: [Engine-devel] Proposal for new commit msg design for
> >>>>> engine     commits
> >>>>>>
> >>>>>>
> >>>>>> ----- Original Message -----
> >>>>>>> From: "Alon Bar-Lev" <alonbl at redhat.com>
> >>>>>>> To: "Eyal Edri" <eedri at redhat.com>
> >>>>>>> Cc: "engine-devel" <engine-devel at ovirt.org>, "infra"
> >>>>> <infra at ovirt.org>
> >>>>>>> Sent: Tuesday, July 9, 2013 3:33:57 PM
> >>>>>>> Subject: Re: [Engine-devel] Proposal for new commit msg design for
> >>>>> engine
> >>>>>>>      commits
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> ----- Original Message -----
> >>>>>>>> From: "Eyal Edri" <eedri at redhat.com>
> >>>>>>>> To: "engine-devel" <engine-devel at ovirt.org>
> >>>>>>>> Cc: "infra" <infra at ovirt.org>
> >>>>>>>> Sent: Tuesday, July 9, 2013 12:38:51 PM
> >>>>>>>> Subject: Proposal for new commit msg design for engine commits
> >>>>>>>>
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> You all probably know and familiar with 'ovirt-engine' git hook
> >>>>> for
> >>>>>>>> commit
> >>>>>>>> msg template [1].
> >>>>>>>> this helps understand the general area of the patch in the
> >>>>> project but it
> >>>>>>>> lacks additional info that might
> >>>>>>>> be valuable for scaling automatic tests in Jenkins CI.
> >>>>>>>>
> >>>>>>>> Let me explain:
> >>>>>>>>
> >>>>>>>> Infra team is working hard on expanding oVirt CI infrastructure
> >>>>> and
> >>>>>>>> adding
> >>>>>>>> more tests in jenkins (per commit/patch).
> >>>>>>>> Adding important meta-data per patch can significatly improve
> >>>>> the ability
> >>>>>>>> to
> >>>>>>>> run specific tests for each patch/commit,
> >>>>>>>> and not waste valuable resources on Jenkins jobs that are not
> >>>>> relevant to
> >>>>>>>> the
> >>>>>>>> code in the patch.
> >>>>>>>>
> >>>>>>>> So the idea is to add/expand current metadata per patch, in the
> >>>>> form of:
> >>>>>>>> (either)
> >>>>>>>>   1. expanding current header template to include more data like
> >>>>> 'network'
> >>>>>>>>   ,
> >>>>>>>>   'setup', 'tools', 'virt'
> >>>>>>> Please do not expand header, it is too short anyway.
> >>>>>>>
> >>>>>>>>   2. adding a new label with relevant tags for the patch, called
> >>>>> e.g
> >>>>>>>>   'METADATA: network, rest, virt'
> >>>>>>> Having:
> >>>>>>>
> >>>>>>> CI-Tests: xxx
> >>>>>>> CI-Tests: yyy
> >>>>>>> CI-Tests: zzz
> >>>>>>>
> >>>>>>> Is much better.
> >>>>>> I'm not sure we should have CI-Test - as we might use this for
> >>>>> something else
> >>>>>> besides CI.
> >>>>>> Region_of_Interest as Dan suggests sounds better IMHO.
> >>>>> I don't care how this is to be called.
> >>>>> However, I do not think that commit message is the place for
> >>>>> instructing CI to do anything.
> >>>>> Commit message stays for good, it should contain information that is
> >>>>> required a year from now.
> >>>>> It has nothing to do with tests and such.
> >>>> I agree with Alon here that the Ci informations don't belong in the
> >>>> commit msg.
> >>>> My opinion is that a testcase should know what it covers. This
> >>>> information from the testcase can then be used by any party to determin
> >>>> if the testcase should be run on a specific commit (which yields
> >>>> informations about the changed paths, files, owner, author, etc ...
> >>>> which might be valuable).
> >>> i think you're missing the point here.
> >>> can you explain how do you propose a test case will know "what it
> >>> covers"?
> >>>
> >>> let's take an example:
> >>> let's say a new commit comes from ovirt-engine:
> >>> http://gerrit.ovirt.org/#/c/16668/
> >>> commit msg: "core: Use images instead of volumes at CDA message".
> >>>
> >>> now you have 1000 test cases (could be system or functional test).
> >>> (let's assume that your infra can't support running 1000 tests per
> >>> patch/commit).
> >>>
> >>> Some of these test suits checks network flow, some virt
> >>> (migration/template
> >>> for e.g), some host install, others storage flows and so on... ).
> >>> you have one repo to clone (ovirt-engine, let's keep vdsm a side for a
> >>> min), and to compile the project from for the tests.
> >>>
> >>> now given this scenario, please explain how will you know which test from
> >>> the 1000 you have you'll run on it.
> >>> do you believe that according to the author/path/filename you'll know if
> >>> that patch involves storage or virt scenario?
> >> Hey Eyal,
> >>
> >> Yes - I would at least give it a try to decide automagically what tests
> >> to run by looking at the change.
> >> The main motivation for this is to not add another things which the
> >> committer needs to take care of and IMO we humans tend to fail (at some
> >> point) on those boring tasks like adding correct metadata (let it be a
> >> typo or just not adding the correct testsuites/topis to be run).
> > this process can be fully automatic via gerrit hooks & templates:
> >
> > typos or mistakes can be easily handles by gerrit hooks to help the
> > committer fix the tags.
> > as mentions previously, this logic can be done by the project maintainer
> > and enforced by a template or hook.
> >
> > so for example - if someone writes a patch with patch header
> > "webadmin:...."  ,
> > then the tags he'll get to choose from are only relevant to ui/ux.
> >
> > so the only task a committer will have to do is to verify the default tags
> > are relevant.
> >
> > i don't believe this is too much to ask for, considering the huge benefit
> > that we'll get
> > (stable code, less bugs, less ci breakage, mapping of specific code to
> > relevant topic, statistics.. etc..)
> >
> >> But let's get back to your example.
> >> Basically we can use the path and filename to determin what testsuite to
> >> run.
> >> Testsuites could be bound to paths and/or filenames and/or regexes being
> >> matched against the full filename.
> >> Another approach would be to use a java package dependency tree to
> >> determine which classes are directly and indirectly affected by a
> >> change. This information can then be used to also build a set of
> >> testsuites to be run. For example:
> >> World uses Ocean uses Wale uses Cell - if Wale changes, we'll surely
> >> want to run the testsuites assigned to the classes higher up in the
> >> dependency chain (World and Ocean).
> >>
> >> For the concrete example above: Maybe there is a spell checker testcase
> >> which could be bound to the filename glob pattern *.properties.
> >>
> >>> i don't think there's an alternative to a metadata to assist mapping the
> >>> patch to a relevant "topic" in the code.
> >>> whether it exists as a git note or a label in the commit, that's another
> >>> matter and probably less important.
> >> The idea is to use the path/filename and dependency tree information to
> >> model these topics. Example:
> >> WaterTestsuite(Topic):
> >>    regex_in_change: .*\.fish
> >>    glob_in_change: src/classes/ocean/*.java
> >>    path_in_change: src/classes/water.java
> >>    change_affects_depency_of: WaterClass
> > I'm not familiar that much with the names of the classes and filenames, but
> > it sounds to me like an error prone process
> > and very complex to start going through all the classes and file names and
> > mapping them to a certain project/component.
> > sounds like we'll have to enforce a naming convention for every new
> > file/path/class name that won't break that magic
> > detection.
> >
> > sure there are exceptions that will work probably, like "anything under
> > packaging/, should trigger the 'engine-setup' or 'engine-upgrade' tests,
> > but imo, it is not so easy with other components.
> >
> > if something will help, it will be splitting up ovirt-engine into subject
> > projects (different git)
> >
> > Eyal.
> 
> I think some valid points were raised in this thread, and I feel we all
> agree regarding the need for such a mechanism.
> regarding mapping of different areas in the code using metadata, i think
> this approach worth trying, it'll increase ownership and area of
> responsibility within our code and hopefully provide us the
> functionality we are looking for.
> we can start doing the obvious mapping, after-which the responsibility
> of each team/maintainer to assign a file to a person and define the
> specific functional areas in it.
> 
> Moran.
> 
> >
> >> But surely labels or meta-data in the commit msg are quicker to
> >> implement.
> >>
> >> - fabian
> >>
> >>> eyal.
> >>>
> >>>> - fabian
> >>>>
> >>>> _______________________________________________
> >>>> Engine-devel mailing list
> >>>> Engine-devel at ovirt.org
> >>>> http://lists.ovirt.org/mailman/listinfo/engine-devel
> >>>>
> >>
> >>
> > _______________________________________________
> > Infra mailing list
> > Infra at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/infra
> 
>

More information about the Infra mailing list