Re: [Freeipa-devel] Linking tickets in the commit messages
On Thu, Sep 17, 2015 at 01:47:29PM +0200, Tomas Babej wrote: > also guilty of this mishap. This poses certain problems, as we're trying > to automate the bookkeeping and pushing-related processes with ipatool [1]. [...] > This would mean: > * Patches fixing an issue described only in BZ (rare issue) would need > to create a Trac ticket referencing the BZ > * Patches fixing an issue not tracked in Trac nor BZ would need to file > a ticket in Trac and reference it On Thu, Sep 17, 2015 at 02:13:20PM +0200, Tomas Babej wrote: > > which does not provide instructions in the case when the corresponding > ticket does not exist. If the fix is for issue not in trac, presumably some trivial change like a typo in some message, how does a commit without ticket link break ipatool? I understand that having reference to an issue in bugzilla in the commit is useful because it's then possible to find commits related to that issue, which is already tracked somewhere. Still, even for bugzilla, you could possibly just use the bugzilla URL. But if the issue is so small that noone really thought about filing it before, what's the purpose of creating one? We don't seem to suffer from the lack of tickets. -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 17.9.2015 17:42, Alexander Bokovoy wrote: > On Thu, 17 Sep 2015, Rob Crittenden wrote: >> Alexander Bokovoy wrote: >>> On Thu, 17 Sep 2015, Jakub Hrozek wrote: On Thu, Sep 17, 2015 at 03:55:35PM +0300, Alexander Bokovoy wrote: > >Speaking as IPA package maitainer in RHEL, I would like to have ticket > >link in every commit in maintenance branches. If a commit goes to the > >master branch only, I'm OK with it not having a ticket link. So that's > >where I would draw the line - if a commit goes into a maintenance > branch, > >it is a reasonable piece of work. > Good suggestion, thanks. We actually have the same with Samba -- *any* > backport to released branches requires a new bug to be opened and > mentioned in the commit message. Seems reasonable for SSSD as well.. Two questions: 1) If you backport from a non-maintenance branch to a maintenance branch, do you also move the ticket? IOW, do you also expect the list of changes to be visible in track, or do you only care about 'auditing' of each commit? >>> Samba clones the bug. FreeIPA does add a comment with the hashes >>> referencing the commits. >>> >>> 2) If another commit (which can be totally unrelated in functionality, just touching the same area of code) needs to be applied before the one you backport, do you add the ticket URL to the prerequisite as well or create a new one? >>> If you have something else applying to a maintenance branch, just do a >>> bug for it. If it is a patch for making the backport more manageable for >>> applying, just have it part of the backport. >>> >> >> I can't quite figure out what problem you're trying to solve here. Why >> not just reference the original ticket in the backported commit? What >> value, except perhaps in ticket reports, does this bring? > It may, perhaps, be irrelevant to current FreeIPA state of integration > with different distributions and products, but in Samba case a version > often lives beyond what is supported by upstream, so a good trail of how > backports landed in a released branch is often reused by various ISVs to > track their own long term product branches. Samba code did undergo large > changes in last ten years to the point that sometimes these backports > are completely separate patchsets on their own, with own life and > original bugs fixes/design decisions might not apply anymore exactly. > Having a bug to record decisions made some time ago specifically to the > branch helps in future when another person comes with a bug due to the > backport -- not so rare, unfortunately, as sometimes an obscure side > effect might change what others are caring about. > > I guess it also a sign of a Trac's and bugzilla's failure to allow > attaching a single bug/ticket to multiple releases in a clear way so > that people have to clone bugs to different streams to represent the > same story in different contexts. Well, I would say that it is our failure to use Trac :-) Nothing prevents us from using a separate field for tracking branches in which the patch landed, or something even more precise. E.g. bind-dyndb-ldap Trac has field 'fixedby' which contains hashes of commits fixing given ticket. Nothing prevents me from putting multiple hashes from multiple branches to that field, and I do that. Then, git describe --contains nicely shows which released version (tag) contains which commit etc. The main question is if it worth the effort because bookkeeping may easily consume more resources than the benefit, especially for small fixes. Should we create tickets for typo fix or other non-essential change just because it is applicable to all branches? I do not think so. I very much agree with Jan Pazdiora's note from other part of the thread: > But if the issue is so small that noone really thought about filing > it before, what's the purpose of creating one? We don't seem to > suffer from the lack of tickets. -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
Hi, though I'm not a fellow developer, I'd like to point out that the habit you mentioned (putting a link into commit message) is indeed documented on wiki [1]. It's just on a spot where it's not that much visible. Sorry if I misunderstood the point of your email. Lenka [1] http://www.freeipa.org/page/Contribute/Patch_Format On 09/17/2015 01:47 PM, Tomas Babej wrote: Hi fellow developers, more or less we tend to stick to the tradition of linking Trac tickets to the commit messages of the patches we send to the list. However, every now and then, a patch lands on the list, which is either linked to a BZ or does not contain any link at all. Admittedly, I am also guilty of this mishap. This poses certain problems, as we're trying to automate the bookkeeping and pushing-related processes with ipatool [1]. Nevertheless, this useful habit is not formally agreed upon by developers nor documented in our wiki [2]. I'd suggest we add it there, if we come to such consensus. This would mean: * Patches fixing an issue described only in BZ (rare issue) would need to create a Trac ticket referencing the BZ * Patches fixing an issue not tracked in Trac nor BZ would need to file a ticket in Trac and reference it Thoughts? [1] https://github.com/freeipa/freeipa-tools/blob/master/ipatool [2] http://www.freeipa.org/page/Contribute/Code -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 09/17/2015 01:47 PM, Tomas Babej wrote: > Hi fellow developers, > > more or less we tend to stick to the tradition of linking Trac tickets > to the commit messages of the patches we send to the list. > > However, every now and then, a patch lands on the list, which is either > linked to a BZ or does not contain any link at all. Admittedly, I am > also guilty of this mishap. This poses certain problems, as we're trying > to automate the bookkeeping and pushing-related processes with ipatool [1]. > > Nevertheless, this useful habit is not formally agreed upon by > developers nor documented in our wiki [2]. I'd suggest we add it there, > if we come to such consensus. > > This would mean: > * Patches fixing an issue described only in BZ (rare issue) would need > to create a Trac ticket referencing the BZ +1 > * Patches fixing an issue not tracked in Trac nor BZ would need to file > a ticket in Trac and reference it I am not sure we are there yet. For typos and small fixes, I do not think we need to create a hard requirement for a Trac ticket. But for patches that you want to be considered for say backports to downstream releases, it is better to have the ticket with the right metadata and collection of the right hashes that the downstream release can digest. > > Thoughts? > > [1] https://github.com/freeipa/freeipa-tools/blob/master/ipatool > [2] http://www.freeipa.org/page/Contribute/Code > -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 01:47 PM, Tomas Babej wrote: Hi fellow developers, more or less we tend to stick to the tradition of linking Trac tickets to the commit messages of the patches we send to the list. However, every now and then, a patch lands on the list, which is either linked to a BZ or does not contain any link at all. Admittedly, I am also guilty of this mishap. This poses certain problems, as we're trying to automate the bookkeeping and pushing-related processes with ipatool [1]. Nevertheless, this useful habit is not formally agreed upon by developers nor documented in our wiki [2]. I'd suggest we add it there, if we come to such consensus. This would mean: * Patches fixing an issue described only in BZ (rare issue) would need to create a Trac ticket referencing the BZ +1 * Patches fixing an issue not tracked in Trac nor BZ would need to file a ticket in Trac and reference it I am not sure we are there yet. For typos and small fixes, I do not think we need to create a hard requirement for a Trac ticket. But for patches that you want to be considered for say backports to downstream releases, it is better to have the ticket with the right metadata and collection of the right hashes that the downstream release can digest. Yes, please do a ticket per a changeset. -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 09/17/2015 02:00 PM, Alexander Bokovoy wrote: > On Thu, 17 Sep 2015, Martin Kosek wrote: >> On 09/17/2015 01:47 PM, Tomas Babej wrote: >>> Hi fellow developers, >>> >>> more or less we tend to stick to the tradition of linking Trac tickets >>> to the commit messages of the patches we send to the list. >>> >>> However, every now and then, a patch lands on the list, which is either >>> linked to a BZ or does not contain any link at all. Admittedly, I am >>> also guilty of this mishap. This poses certain problems, as we're trying >>> to automate the bookkeeping and pushing-related processes with ipatool [1]. >>> >>> Nevertheless, this useful habit is not formally agreed upon by >>> developers nor documented in our wiki [2]. I'd suggest we add it there, >>> if we come to such consensus. >>> >>> This would mean: >>> * Patches fixing an issue described only in BZ (rare issue) would need >>> to create a Trac ticket referencing the BZ >> >> +1 >> >>> * Patches fixing an issue not tracked in Trac nor BZ would need to file >>> a ticket in Trac and reference it >> >> I am not sure we are there yet. For typos and small fixes, I do not think we >> need to create a hard requirement for a Trac ticket. But for patches that you >> want to be considered for say backports to downstream releases, it is better >> to >> have the ticket with the right metadata and collection of the right hashes >> that >> the downstream release can digest. > Yes, please do a ticket per a changeset. Are you agreeing now to me, i.e. do not require tickets for trivial fixes or to Tomas' proposal - require ticket for *all* patches? -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 09/17/2015 01:53 PM, Martin Kosek wrote: On 09/17/2015 01:47 PM, Tomas Babej wrote: Hi fellow developers, more or less we tend to stick to the tradition of linking Trac tickets to the commit messages of the patches we send to the list. However, every now and then, a patch lands on the list, which is either linked to a BZ or does not contain any link at all. Admittedly, I am also guilty of this mishap. This poses certain problems, as we're trying to automate the bookkeeping and pushing-related processes with ipatool [1]. Nevertheless, this useful habit is not formally agreed upon by developers nor documented in our wiki [2]. I'd suggest we add it there, if we come to such consensus. This would mean: * Patches fixing an issue described only in BZ (rare issue) would need to create a Trac ticket referencing the BZ +1 +1 * Patches fixing an issue not tracked in Trac nor BZ would need to file a ticket in Trac and reference it I am not sure we are there yet. For typos and small fixes, I do not think we need to create a hard requirement for a Trac ticket. But for patches that you want to be considered for say backports to downstream releases, it is better to have the ticket with the right metadata and collection of the right hashes that the downstream release can digest. +1 Thoughts? [1] https://github.com/freeipa/freeipa-tools/blob/master/ipatool [2] http://www.freeipa.org/page/Contribute/Code -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 09/17/2015 01:53 PM, Martin Kosek wrote: > On 09/17/2015 01:47 PM, Tomas Babej wrote: >> Hi fellow developers, >> >> more or less we tend to stick to the tradition of linking Trac tickets >> to the commit messages of the patches we send to the list. >> >> However, every now and then, a patch lands on the list, which is either >> linked to a BZ or does not contain any link at all. Admittedly, I am >> also guilty of this mishap. This poses certain problems, as we're trying >> to automate the bookkeeping and pushing-related processes with ipatool [1]. >> >> Nevertheless, this useful habit is not formally agreed upon by >> developers nor documented in our wiki [2]. I'd suggest we add it there, >> if we come to such consensus. >> >> This would mean: >> * Patches fixing an issue described only in BZ (rare issue) would need >> to create a Trac ticket referencing the BZ > > +1 > >> * Patches fixing an issue not tracked in Trac nor BZ would need to file >> a ticket in Trac and reference it > > I am not sure we are there yet. For typos and small fixes, I do not think we > need to create a hard requirement for a Trac ticket. But for patches that you > want to be considered for say backports to downstream releases, it is better > to > have the ticket with the right metadata and collection of the right hashes > that > the downstream release can digest. I agree it might be a good idea to be less harsh on the trivial patches, however, how do we draw the line? This puts the decision burden on the contributor who might not have the necessary insight and/or information. I'd suggest we go this route, I don't want to put more obstacles for new contributors and small patches. However, then the regular FreeIPA committers need to notify the new contributors if their patch crosses the triviality barrier and warrants a ticket :) -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 02:00 PM, Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 01:47 PM, Tomas Babej wrote: Hi fellow developers, more or less we tend to stick to the tradition of linking Trac tickets to the commit messages of the patches we send to the list. However, every now and then, a patch lands on the list, which is either linked to a BZ or does not contain any link at all. Admittedly, I am also guilty of this mishap. This poses certain problems, as we're trying to automate the bookkeeping and pushing-related processes with ipatool [1]. Nevertheless, this useful habit is not formally agreed upon by developers nor documented in our wiki [2]. I'd suggest we add it there, if we come to such consensus. This would mean: * Patches fixing an issue described only in BZ (rare issue) would need to create a Trac ticket referencing the BZ +1 * Patches fixing an issue not tracked in Trac nor BZ would need to file a ticket in Trac and reference it I am not sure we are there yet. For typos and small fixes, I do not think we need to create a hard requirement for a Trac ticket. But for patches that you want to be considered for say backports to downstream releases, it is better to have the ticket with the right metadata and collection of the right hashes that the downstream release can digest. Yes, please do a ticket per a changeset. Are you agreeing now to me, i.e. do not require tickets for trivial fixes or to Tomas' proposal - require ticket for *all* patches? I think we should have tickets for reasonable pieces of work. The problem is always in identifying what does 'reasonable' mean. A single-line fix may be an important CVE fix or a key to an important bugfix. Still, there should be a context to fit, thus a ticket. -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On Thu, 17 Sep 2015, Tomas Babej wrote: On 09/17/2015 01:53 PM, Martin Kosek wrote: On 09/17/2015 01:47 PM, Tomas Babej wrote: Hi fellow developers, more or less we tend to stick to the tradition of linking Trac tickets to the commit messages of the patches we send to the list. However, every now and then, a patch lands on the list, which is either linked to a BZ or does not contain any link at all. Admittedly, I am also guilty of this mishap. This poses certain problems, as we're trying to automate the bookkeeping and pushing-related processes with ipatool [1]. Nevertheless, this useful habit is not formally agreed upon by developers nor documented in our wiki [2]. I'd suggest we add it there, if we come to such consensus. This would mean: * Patches fixing an issue described only in BZ (rare issue) would need to create a Trac ticket referencing the BZ +1 * Patches fixing an issue not tracked in Trac nor BZ would need to file a ticket in Trac and reference it I am not sure we are there yet. For typos and small fixes, I do not think we need to create a hard requirement for a Trac ticket. But for patches that you want to be considered for say backports to downstream releases, it is better to have the ticket with the right metadata and collection of the right hashes that the downstream release can digest. I agree it might be a good idea to be less harsh on the trivial patches, however, how do we draw the line? This puts the decision burden on the contributor who might not have the necessary insight and/or information. I'd suggest we go this route, I don't want to put more obstacles for new contributors and small patches. However, then the regular FreeIPA committers need to notify the new contributors if their patch crosses the triviality barrier and warrants a ticket :) Yes, a reviewer should be able to file a ticket. ;) -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 09/17/2015 01:55 PM, Lenka Doudova wrote: > Hi, > > though I'm not a fellow developer, I'd like to point out that the habit $ git shortlog | grep Lenka tells me otherwise. This thread is applicable to everybody contributing to the FreeIPA git repo, so maybe developer was a poor choice of wording. > you mentioned (putting a link into commit message) is indeed documented > on wiki [1]. Indeed, I missed that. However, the cited page only states: "If the patch adresses a ticket in Trac, the last line of the commit should be the URL to that ticket." which does not provide instructions in the case when the corresponding ticket does not exist. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 17.9.2015 14:08, Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 02:00 PM, Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 01:47 PM, Tomas Babej wrote: Hi fellow developers, more or less we tend to stick to the tradition of linking Trac tickets to the commit messages of the patches we send to the list. However, every now and then, a patch lands on the list, which is either linked to a BZ or does not contain any link at all. Admittedly, I am also guilty of this mishap. This poses certain problems, as we're trying to automate the bookkeeping and pushing-related processes with ipatool [1]. Nevertheless, this useful habit is not formally agreed upon by developers nor documented in our wiki [2]. I'd suggest we add it there, if we come to such consensus. This would mean: * Patches fixing an issue described only in BZ (rare issue) would need to create a Trac ticket referencing the BZ +1 * Patches fixing an issue not tracked in Trac nor BZ would need to file a ticket in Trac and reference it I am not sure we are there yet. For typos and small fixes, I do not think we need to create a hard requirement for a Trac ticket. But for patches that you want to be considered for say backports to downstream releases, it is better to have the ticket with the right metadata and collection of the right hashes that the downstream release can digest. Yes, please do a ticket per a changeset. Are you agreeing now to me, i.e. do not require tickets for trivial fixes or to Tomas' proposal - require ticket for *all* patches? I think we should have tickets for reasonable pieces of work. The problem is always in identifying what does 'reasonable' mean. A single-line fix may be an important CVE fix or a key to an important bugfix. Still, there should be a context to fit, thus a ticket. Speaking as IPA package maitainer in RHEL, I would like to have ticket link in every commit in maintenance branches. If a commit goes to the master branch only, I'm OK with it not having a ticket link. So that's where I would draw the line - if a commit goes into a maintenance branch, it is a reasonable piece of work. -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On Thu, 17 Sep 2015, Jan Cholasta wrote: On 17.9.2015 14:08, Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 02:00 PM, Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 01:47 PM, Tomas Babej wrote: Hi fellow developers, more or less we tend to stick to the tradition of linking Trac tickets to the commit messages of the patches we send to the list. However, every now and then, a patch lands on the list, which is either linked to a BZ or does not contain any link at all. Admittedly, I am also guilty of this mishap. This poses certain problems, as we're trying to automate the bookkeeping and pushing-related processes with ipatool [1]. Nevertheless, this useful habit is not formally agreed upon by developers nor documented in our wiki [2]. I'd suggest we add it there, if we come to such consensus. This would mean: * Patches fixing an issue described only in BZ (rare issue) would need to create a Trac ticket referencing the BZ +1 * Patches fixing an issue not tracked in Trac nor BZ would need to file a ticket in Trac and reference it I am not sure we are there yet. For typos and small fixes, I do not think we need to create a hard requirement for a Trac ticket. But for patches that you want to be considered for say backports to downstream releases, it is better to have the ticket with the right metadata and collection of the right hashes that the downstream release can digest. Yes, please do a ticket per a changeset. Are you agreeing now to me, i.e. do not require tickets for trivial fixes or to Tomas' proposal - require ticket for *all* patches? I think we should have tickets for reasonable pieces of work. The problem is always in identifying what does 'reasonable' mean. A single-line fix may be an important CVE fix or a key to an important bugfix. Still, there should be a context to fit, thus a ticket. Speaking as IPA package maitainer in RHEL, I would like to have ticket link in every commit in maintenance branches. If a commit goes to the master branch only, I'm OK with it not having a ticket link. So that's where I would draw the line - if a commit goes into a maintenance branch, it is a reasonable piece of work. Good suggestion, thanks. We actually have the same with Samba -- *any* backport to released branches requires a new bug to be opened and mentioned in the commit message. -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 09/17/2015 04:47 PM, Jakub Hrozek wrote: On Thu, Sep 17, 2015 at 03:55:35PM +0300, Alexander Bokovoy wrote: Speaking as IPA package maitainer in RHEL, I would like to have ticket link in every commit in maintenance branches. If a commit goes to the master branch only, I'm OK with it not having a ticket link. So that's where I would draw the line - if a commit goes into a maintenance branch, it is a reasonable piece of work. Good suggestion, thanks. We actually have the same with Samba -- *any* backport to released branches requires a new bug to be opened and mentioned in the commit message. Seems reasonable for SSSD as well.. Two questions: 1) If you backport from a non-maintenance branch to a maintenance branch, do you also move the ticket? IOW, do you also expect the list of changes to be visible in track, or do you only care about 'auditing' of each commit? This cases happens rarely in FreeIPA upstream. Usually the patch is directly developed for a maintenance branch and all branches after that. E.g. a patch for 4.1.5 would go to ipa-4-1, ipa-4-2 and master branch. It would be in FreeIPA 4.1.5 milestone. Commits for each branch are recorded in the ticket. 2) If another commit (which can be totally unrelated in functionality, just touching the same area of code) needs to be applied before the one you backport, do you add the ticket URL to the prerequisite as well or create a new one? If a backport happens much later, IMHO the proper way would be to create a separate ticket for the backport and reference the original ticket(s) and record all commits, even the prerequisites. -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
Alexander Bokovoy wrote: > On Thu, 17 Sep 2015, Jakub Hrozek wrote: >> On Thu, Sep 17, 2015 at 03:55:35PM +0300, Alexander Bokovoy wrote: >>> >Speaking as IPA package maitainer in RHEL, I would like to have ticket >>> >link in every commit in maintenance branches. If a commit goes to the >>> >master branch only, I'm OK with it not having a ticket link. So that's >>> >where I would draw the line - if a commit goes into a maintenance >>> branch, >>> >it is a reasonable piece of work. >>> Good suggestion, thanks. We actually have the same with Samba -- *any* >>> backport to released branches requires a new bug to be opened and >>> mentioned in the commit message. >> >> Seems reasonable for SSSD as well.. >> >> Two questions: >>1) If you backport from a non-maintenance branch to a maintenance >> branch, do you also move the ticket? IOW, do you also expect the >> list of changes to be visible in track, or do you only care about >> 'auditing' of each commit? > Samba clones the bug. FreeIPA does add a comment with the hashes > referencing the commits. > > >>2) If another commit (which can be totally unrelated in >> functionality, just touching the same area of code) needs to be >> applied before the one you backport, do you add the ticket URL >> to the prerequisite as well or create a new one? > If you have something else applying to a maintenance branch, just do a > bug for it. If it is a patch for making the backport more manageable for > applying, just have it part of the backport. > I can't quite figure out what problem you're trying to solve here. Why not just reference the original ticket in the backported commit? What value, except perhaps in ticket reports, does this bring? rob -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On Thu, 2015-09-17 at 14:07 +0200, Tomas Babej wrote: > > On 09/17/2015 01:53 PM, Martin Kosek wrote: > > On 09/17/2015 01:47 PM, Tomas Babej wrote: > >> Hi fellow developers, > >> > >> more or less we tend to stick to the tradition of linking Trac tickets > >> to the commit messages of the patches we send to the list. > >> > >> However, every now and then, a patch lands on the list, which is either > >> linked to a BZ or does not contain any link at all. Admittedly, I am > >> also guilty of this mishap. This poses certain problems, as we're trying > >> to automate the bookkeeping and pushing-related processes with ipatool [1]. > >> > >> Nevertheless, this useful habit is not formally agreed upon by > >> developers nor documented in our wiki [2]. I'd suggest we add it there, > >> if we come to such consensus. > >> > >> This would mean: > >> * Patches fixing an issue described only in BZ (rare issue) would need > >> to create a Trac ticket referencing the BZ > > > > +1 > > > >> * Patches fixing an issue not tracked in Trac nor BZ would need to file > >> a ticket in Trac and reference it > > > > I am not sure we are there yet. For typos and small fixes, I do not think we > > need to create a hard requirement for a Trac ticket. But for patches that > > you > > want to be considered for say backports to downstream releases, it is > > better to > > have the ticket with the right metadata and collection of the right hashes > > that > > the downstream release can digest. > > I agree it might be a good idea to be less harsh on the trivial patches, > however, how do we draw the line? This puts the decision burden on the > contributor who might not have the necessary insight and/or information. > > I'd suggest we go this route, I don't want to put more obstacles for new > contributors and small patches. However, then the regular FreeIPA > committers need to notify the new contributors if their patch crosses > the triviality barrier and warrants a ticket :) Given you push through a tool, maybe you can teach the tool to open a new ticket if one is not referenced ? Simo. -- Simo Sorce * Red Hat, Inc * New York -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On Thu, 17 Sep 2015, Rob Crittenden wrote: Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Jakub Hrozek wrote: On Thu, Sep 17, 2015 at 03:55:35PM +0300, Alexander Bokovoy wrote: >Speaking as IPA package maitainer in RHEL, I would like to have ticket >link in every commit in maintenance branches. If a commit goes to the >master branch only, I'm OK with it not having a ticket link. So that's >where I would draw the line - if a commit goes into a maintenance branch, >it is a reasonable piece of work. Good suggestion, thanks. We actually have the same with Samba -- *any* backport to released branches requires a new bug to be opened and mentioned in the commit message. Seems reasonable for SSSD as well.. Two questions: 1) If you backport from a non-maintenance branch to a maintenance branch, do you also move the ticket? IOW, do you also expect the list of changes to be visible in track, or do you only care about 'auditing' of each commit? Samba clones the bug. FreeIPA does add a comment with the hashes referencing the commits. 2) If another commit (which can be totally unrelated in functionality, just touching the same area of code) needs to be applied before the one you backport, do you add the ticket URL to the prerequisite as well or create a new one? If you have something else applying to a maintenance branch, just do a bug for it. If it is a patch for making the backport more manageable for applying, just have it part of the backport. I can't quite figure out what problem you're trying to solve here. Why not just reference the original ticket in the backported commit? What value, except perhaps in ticket reports, does this bring? It may, perhaps, be irrelevant to current FreeIPA state of integration with different distributions and products, but in Samba case a version often lives beyond what is supported by upstream, so a good trail of how backports landed in a released branch is often reused by various ISVs to track their own long term product branches. Samba code did undergo large changes in last ten years to the point that sometimes these backports are completely separate patchsets on their own, with own life and original bugs fixes/design decisions might not apply anymore exactly. Having a bug to record decisions made some time ago specifically to the branch helps in future when another person comes with a bug due to the backport -- not so rare, unfortunately, as sometimes an obscure side effect might change what others are caring about. I guess it also a sign of a Trac's and bugzilla's failure to allow attaching a single bug/ticket to multiple releases in a clear way so that people have to clone bugs to different streams to represent the same story in different contexts. -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 17.9.2015 15:02, Milan Kubík wrote: On 09/17/2015 02:51 PM, Jan Cholasta wrote: On 17.9.2015 14:08, Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 02:00 PM, Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 01:47 PM, Tomas Babej wrote: Hi fellow developers, more or less we tend to stick to the tradition of linking Trac tickets to the commit messages of the patches we send to the list. However, every now and then, a patch lands on the list, which is either linked to a BZ or does not contain any link at all. Admittedly, I am also guilty of this mishap. This poses certain problems, as we're trying to automate the bookkeeping and pushing-related processes with ipatool [1]. Nevertheless, this useful habit is not formally agreed upon by developers nor documented in our wiki [2]. I'd suggest we add it there, if we come to such consensus. This would mean: * Patches fixing an issue described only in BZ (rare issue) would need to create a Trac ticket referencing the BZ +1 * Patches fixing an issue not tracked in Trac nor BZ would need to file a ticket in Trac and reference it I am not sure we are there yet. For typos and small fixes, I do not think we need to create a hard requirement for a Trac ticket. But for patches that you want to be considered for say backports to downstream releases, it is better to have the ticket with the right metadata and collection of the right hashes that the downstream release can digest. Yes, please do a ticket per a changeset. Are you agreeing now to me, i.e. do not require tickets for trivial fixes or to Tomas' proposal - require ticket for *all* patches? I think we should have tickets for reasonable pieces of work. The problem is always in identifying what does 'reasonable' mean. A single-line fix may be an important CVE fix or a key to an important bugfix. Still, there should be a context to fit, thus a ticket. Speaking as IPA package maitainer in RHEL, I would like to have ticket link in every commit in maintenance branches. If a commit goes to the master branch only, I'm OK with it not having a ticket link. So that's where I would draw the line - if a commit goes into a maintenance branch, it is a reasonable piece of work. Hi, in general, which ticket should be referenced from a commit that implements tests? It can happen that the tests cover a set of tickets. What should we do then? I would think file a new ticket with component set to "Tests" and reference it in the commit. Cheers, Milan -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On Thu, 17 Sep 2015, Jakub Hrozek wrote: On Thu, Sep 17, 2015 at 03:55:35PM +0300, Alexander Bokovoy wrote: >Speaking as IPA package maitainer in RHEL, I would like to have ticket >link in every commit in maintenance branches. If a commit goes to the >master branch only, I'm OK with it not having a ticket link. So that's >where I would draw the line - if a commit goes into a maintenance branch, >it is a reasonable piece of work. Good suggestion, thanks. We actually have the same with Samba -- *any* backport to released branches requires a new bug to be opened and mentioned in the commit message. Seems reasonable for SSSD as well.. Two questions: 1) If you backport from a non-maintenance branch to a maintenance branch, do you also move the ticket? IOW, do you also expect the list of changes to be visible in track, or do you only care about 'auditing' of each commit? Samba clones the bug. FreeIPA does add a comment with the hashes referencing the commits. 2) If another commit (which can be totally unrelated in functionality, just touching the same area of code) needs to be applied before the one you backport, do you add the ticket URL to the prerequisite as well or create a new one? If you have something else applying to a maintenance branch, just do a bug for it. If it is a patch for making the backport more manageable for applying, just have it part of the backport. -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On Thu, Sep 17, 2015 at 03:55:35PM +0300, Alexander Bokovoy wrote: > >Speaking as IPA package maitainer in RHEL, I would like to have ticket > >link in every commit in maintenance branches. If a commit goes to the > >master branch only, I'm OK with it not having a ticket link. So that's > >where I would draw the line - if a commit goes into a maintenance branch, > >it is a reasonable piece of work. > Good suggestion, thanks. We actually have the same with Samba -- *any* > backport to released branches requires a new bug to be opened and > mentioned in the commit message. Seems reasonable for SSSD as well.. Two questions: 1) If you backport from a non-maintenance branch to a maintenance branch, do you also move the ticket? IOW, do you also expect the list of changes to be visible in track, or do you only care about 'auditing' of each commit? 2) If another commit (which can be totally unrelated in functionality, just touching the same area of code) needs to be applied before the one you backport, do you add the ticket URL to the prerequisite as well or create a new one? -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Linking tickets in the commit messages
On 09/17/2015 02:51 PM, Jan Cholasta wrote: On 17.9.2015 14:08, Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 02:00 PM, Alexander Bokovoy wrote: On Thu, 17 Sep 2015, Martin Kosek wrote: On 09/17/2015 01:47 PM, Tomas Babej wrote: Hi fellow developers, more or less we tend to stick to the tradition of linking Trac tickets to the commit messages of the patches we send to the list. However, every now and then, a patch lands on the list, which is either linked to a BZ or does not contain any link at all. Admittedly, I am also guilty of this mishap. This poses certain problems, as we're trying to automate the bookkeeping and pushing-related processes with ipatool [1]. Nevertheless, this useful habit is not formally agreed upon by developers nor documented in our wiki [2]. I'd suggest we add it there, if we come to such consensus. This would mean: * Patches fixing an issue described only in BZ (rare issue) would need to create a Trac ticket referencing the BZ +1 * Patches fixing an issue not tracked in Trac nor BZ would need to file a ticket in Trac and reference it I am not sure we are there yet. For typos and small fixes, I do not think we need to create a hard requirement for a Trac ticket. But for patches that you want to be considered for say backports to downstream releases, it is better to have the ticket with the right metadata and collection of the right hashes that the downstream release can digest. Yes, please do a ticket per a changeset. Are you agreeing now to me, i.e. do not require tickets for trivial fixes or to Tomas' proposal - require ticket for *all* patches? I think we should have tickets for reasonable pieces of work. The problem is always in identifying what does 'reasonable' mean. A single-line fix may be an important CVE fix or a key to an important bugfix. Still, there should be a context to fit, thus a ticket. Speaking as IPA package maitainer in RHEL, I would like to have ticket link in every commit in maintenance branches. If a commit goes to the master branch only, I'm OK with it not having a ticket link. So that's where I would draw the line - if a commit goes into a maintenance branch, it is a reasonable piece of work. Hi, in general, which ticket should be referenced from a commit that implements tests? It can happen that the tests cover a set of tickets. What should we do then? Cheers, Milan -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
