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

From: "Eyal Edri" <eedri(a)redhat.com&gt; To: "Alon Bar-Lev" <alonbl(a)redhat.com&gt; Cc: "infra" <infra(a)ovirt.org&gt;, "engine-devel" <engine-devel(a)ovirt.org&gt;, "Fabian Deutsch" <fabiand(a)redhat.com&gt; Sent: Saturday, July 20, 2013 9:41:56 PM Subject: Re: [Engine-devel] Proposal for new commit msg design for engine commits 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(a)redhat.com&gt; > To: "Eyal Edri" <eedri(a)redhat.com&gt; > Cc: "infra" <infra(a)ovirt.org&gt;, "engine-devel" <engine-devel(a)ovirt.org&gt;, > "Fabian Deutsch" <fabiand(a)redhat.com&gt; > 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(a)redhat.com&gt; > > To: "infra" <infra(a)ovirt.org&gt; > > Cc: "engine-devel" <engine-devel(a)ovirt.org&gt;, "Fabian Deutsch" > > <fabiand(a)redhat.com&gt; > > 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(a)redhat.com&gt; > > > To: "Eyal Edri" <eedri(a)redhat.com&gt; > > > Cc: "Fabian Deutsch" <fabiand(a)redhat.com&gt;, "engine-devel" > > > <engine-devel(a)ovirt.org&gt;, "infra" <infra(a)ovirt.org&gt; > > > 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(a)redhat.com&gt; > > > >> To: "Eyal Edri" <eedri(a)redhat.com&gt; > > > >> Cc: "Alon Bar-Lev" <alonbl(a)redhat.com&gt;, "engine-devel" > > > >> <engine-devel(a)ovirt.org&gt;, "infra" <infra(a)ovirt.org&gt; > > > >> 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(a)redhat.com&gt; > > > >>>> To: "Alon Bar-Lev" <alonbl(a)redhat.com&gt; > > > >>>> Cc: "engine-devel" <engine-devel(a)ovirt.org&gt;, "infra" > > > >>>> <infra(a)ovirt.org&gt; > > > >>>> 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(a)redhat.com&gt; > > > >>>>>> To: "Alon Bar-Lev" <alonbl(a)redhat.com&gt; > > > >>>>>> Cc: "Eyal Edri" <eedri(a)redhat.com&gt;, "engine-devel" > > > >>>>> <engine-devel(a)ovirt.org&gt;, "infra" <infra(a)ovirt.org&gt; > > > >>>>>> 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(a)redhat.com&gt; > > > >>>>>>> To: "Eyal Edri" <eedri(a)redhat.com&gt; > > > >>>>>>> Cc: "engine-devel" <engine-devel(a)ovirt.org&gt;, "infra" > > > >>>>> <infra(a)ovirt.org&gt; > > > >>>>>>> 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(a)redhat.com&gt; > > > >>>>>>>> To: "engine-devel" <engine-devel(a)ovirt.org&gt; > > > >>>>>>>> Cc: "infra" <infra(a)ovirt.org&gt; > > > >>>>>>>> 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(a)ovirt.org > > > >>>> http://lists.ovirt.org/mailman/listinfo/engine-devel > > > >>>> > > > >> > > > >> > > > > _______________________________________________ > > > > Infra mailing list > > > > Infra(a)ovirt.org > > > > http://lists.ovirt.org/mailman/listinfo/infra > > > > > > > > _______________________________________________ > > Engine-devel mailing list > > Engine-devel(a)ovirt.org > > http://lists.ovirt.org/mailman/listinfo/engine-devel > > >