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

Eyal Edri eedri at redhat.com
Sat Jul 20 18:41:56 UTC 2013 

This change to commit template has nothing to do with CI.
it's a change that should reflect updated components relevance to the commit code.

Nevertheless, i have no problems with your suggestions for metadata per directory to map all ovirt code.
any suggestion how to push it forward? 

Eyal.

----- Original Message -----
> From: "Alon Bar-Lev" <alonbl at redhat.com>
> To: "Eyal Edri" <eedri at redhat.com>
> Cc: "infra" <infra at ovirt.org>, "engine-devel" <engine-devel at ovirt.org>, "Fabian Deutsch" <fabiand at redhat.com>
> Sent: Saturday, July 20, 2013 9:34:13 PM
> Subject: Re: [Engine-devel] Proposal for new commit msg design for engine commits
> 
> 
> 
> ----- Original Message -----
> > From: "Eyal Edri" <eedri at redhat.com>
> > To: "infra" <infra at ovirt.org>
> > Cc: "engine-devel" <engine-devel at ovirt.org>, "Fabian Deutsch"
> > <fabiand at redhat.com>
> > Sent: Saturday, July 20, 2013 8:34:28 PM
> > Subject: Re: [Engine-devel] Proposal for new commit msg design for engine
> > commits
> > 
> > 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)
> 
> Again, and I am sorry, but I disagree of any relationship between commit
> message and CI.
> 
> It will be simple to add metadata to sources, and have CI run tests based on
> actual source change thus probable impact, this way we won't be exposed to
> human errors, nor make commit message unusable for actual history.
> 
> All we need is someone to take ownership of the task of adding metadata to
> source tree.
> 
> As I proposed this can be either within every source using special signature,
> or can be in a directory at special file, for example .ovirt-metadata, and
> have the mapping between source component to relevant tests at a simple text
> file at source root.
> 
> Regards,
> Alon Bar-Lev.
> 
> > 
> > [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
> > > 
> > > 
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > 
>

More information about the Engine-devel mailing list