Re: [Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION
On 12/09/2013 02:50 PM, Martin Kosek wrote: On 12/09/2013 02:35 PM, Simo Sorce wrote: On Mon, 2013-12-09 at 12:39 +0100, Martin Kosek wrote: On 12/09/2013 12:08 PM, Tomas Babej wrote: On 12/05/2013 01:37 PM, Petr Viktorin wrote: Consider this scenario: - Nathaniel submits RADIUS patches that update the API version (from 2.69 to 2.70) - I have ACI patches that also bump the version (from 2.69 to 2.70) - Nathaniel's patches gets accepted - I rebase my ACI patches onto master. Git thinks that the 2.69->2.70 change is already done, so it leaves VERSION unchanged. I can solve this locally by telling Git to not merge VERSION automatically, but I think it would be helpful to add a unique comment to each change so that everyone gets a conflict cases like this. Do you agree? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Makes sense to me. I'd just add a comment so that the purpose of the last change comment is also obvious for the new developer perusing the VERSION file. Maybe something along the lines of: IPA_API_VERSION_MAJOR=2 IPA_API_VERSION_MINOR=70 +# Update the last change entry to enforce conflict on merging two independent branches into master. +# Last change: npmccallum - RADIUS support I don't think this is necessary, IMO "Last change:" is enough as instructions. I spoke with Petr offline, to me it would make bigger sense if we just forbid automatic merging of this line on the git server side (if possible) instead of adding other arbitrary work to our development process. IIRC, Petr3 said it should be possible to do. Except it may not fix the issue, if someone does a rebase on his machine and resubmit a patch to the list w/o noticing the change was effectively dropped. Simo. I thought that the point of the anti-merge protection is to prevent git merging tools to prevent automatic rebase of this particular line and force manual merging, i.e. force increasing the number. When the file is equal on both sides of the merge, Git just uses the common file, and doesn't consider it for merging. So unfortunately, git attributes won't work here; there needs to be another change in the file. I did say otherwise, sorry for that. -- PetrĀ³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION
On 12/09/2013 02:35 PM, Simo Sorce wrote: > On Mon, 2013-12-09 at 12:39 +0100, Martin Kosek wrote: >> On 12/09/2013 12:08 PM, Tomas Babej wrote: >>> On 12/05/2013 01:37 PM, Petr Viktorin wrote: Consider this scenario: - Nathaniel submits RADIUS patches that update the API version (from 2.69 to 2.70) - I have ACI patches that also bump the version (from 2.69 to 2.70) - Nathaniel's patches gets accepted - I rebase my ACI patches onto master. Git thinks that the 2.69->2.70 change is already done, so it leaves VERSION unchanged. I can solve this locally by telling Git to not merge VERSION automatically, but I think it would be helpful to add a unique comment to each change so that everyone gets a conflict cases like this. Do you agree? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel >>> >>> Makes sense to me. >>> >>> I'd just add a comment so that the purpose of the last change comment is >>> also >>> obvious for the new developer perusing the VERSION file. >>> >>> Maybe something along the lines of: >>> >>> >>> IPA_API_VERSION_MAJOR=2 >>> IPA_API_VERSION_MINOR=70 >>> +# Update the last change entry to enforce conflict on merging two >>> independent >>> branches into master. >>> +# Last change: npmccallum - RADIUS support >> >> I spoke with Petr offline, to me it would make bigger sense if we just forbid >> automatic merging of this line on the git server side (if possible) instead >> of >> adding other arbitrary work to our development process. >> >> IIRC, Petr3 said it should be possible to do. > > Except it may not fix the issue, if someone does a rebase on his machine > and resubmit a patch to the list w/o noticing the change was effectively > dropped. > > Simo. I thought that the point of the anti-merge protection is to prevent git merging tools to prevent automatic rebase of this particular line and force manual merging, i.e. force increasing the number. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION
On Mon, 2013-12-09 at 12:39 +0100, Martin Kosek wrote: > On 12/09/2013 12:08 PM, Tomas Babej wrote: > > On 12/05/2013 01:37 PM, Petr Viktorin wrote: > >> Consider this scenario: > >> > >> - Nathaniel submits RADIUS patches that update the API version (from 2.69 > >> to > >> 2.70) > >> - I have ACI patches that also bump the version (from 2.69 to 2.70) > >> - Nathaniel's patches gets accepted > >> - I rebase my ACI patches onto master. Git thinks that the 2.69->2.70 > >> change > >> is already done, so it leaves VERSION unchanged. > >> > >> I can solve this locally by telling Git to not merge VERSION automatically, > >> but I think it would be helpful to add a unique comment to each change so > >> that everyone gets a conflict cases like this. > >> Do you agree? > >> > >> > >> > >> ___ > >> Freeipa-devel mailing list > >> Freeipa-devel@redhat.com > >> https://www.redhat.com/mailman/listinfo/freeipa-devel > > > > Makes sense to me. > > > > I'd just add a comment so that the purpose of the last change comment is > > also > > obvious for the new developer perusing the VERSION file. > > > > Maybe something along the lines of: > > > > > > IPA_API_VERSION_MAJOR=2 > > IPA_API_VERSION_MINOR=70 > > +# Update the last change entry to enforce conflict on merging two > > independent > > branches into master. > > +# Last change: npmccallum - RADIUS support > > I spoke with Petr offline, to me it would make bigger sense if we just forbid > automatic merging of this line on the git server side (if possible) instead of > adding other arbitrary work to our development process. > > IIRC, Petr3 said it should be possible to do. Except it may not fix the issue, if someone does a rebase on his machine and resubmit a patch to the list w/o noticing the change was effectively dropped. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION
On 12/09/2013 12:08 PM, Tomas Babej wrote: > On 12/05/2013 01:37 PM, Petr Viktorin wrote: >> Consider this scenario: >> >> - Nathaniel submits RADIUS patches that update the API version (from 2.69 to >> 2.70) >> - I have ACI patches that also bump the version (from 2.69 to 2.70) >> - Nathaniel's patches gets accepted >> - I rebase my ACI patches onto master. Git thinks that the 2.69->2.70 change >> is already done, so it leaves VERSION unchanged. >> >> I can solve this locally by telling Git to not merge VERSION automatically, >> but I think it would be helpful to add a unique comment to each change so >> that everyone gets a conflict cases like this. >> Do you agree? >> >> >> >> ___ >> Freeipa-devel mailing list >> Freeipa-devel@redhat.com >> https://www.redhat.com/mailman/listinfo/freeipa-devel > > Makes sense to me. > > I'd just add a comment so that the purpose of the last change comment is also > obvious for the new developer perusing the VERSION file. > > Maybe something along the lines of: > > > IPA_API_VERSION_MAJOR=2 > IPA_API_VERSION_MINOR=70 > +# Update the last change entry to enforce conflict on merging two independent > branches into master. > +# Last change: npmccallum - RADIUS support I spoke with Petr offline, to me it would make bigger sense if we just forbid automatic merging of this line on the git server side (if possible) instead of adding other arbitrary work to our development process. IIRC, Petr3 said it should be possible to do. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION
On 12/05/2013 01:37 PM, Petr Viktorin wrote: Consider this scenario: - Nathaniel submits RADIUS patches that update the API version (from 2.69 to 2.70) - I have ACI patches that also bump the version (from 2.69 to 2.70) - Nathaniel's patches gets accepted - I rebase my ACI patches onto master. Git thinks that the 2.69->2.70 change is already done, so it leaves VERSION unchanged. I can solve this locally by telling Git to not merge VERSION automatically, but I think it would be helpful to add a unique comment to each change so that everyone gets a conflict cases like this. Do you agree? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Makes sense to me. I'd just add a comment so that the purpose of the last change comment is also obvious for the new developer perusing the VERSION file. Maybe something along the lines of: IPA_API_VERSION_MAJOR=2 IPA_API_VERSION_MINOR=70 +# Update the last change entry to enforce conflict on merging two independent branches into master. +# Last change: npmccallum - RADIUS support -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION
Consider this scenario: - Nathaniel submits RADIUS patches that update the API version (from 2.69 to 2.70) - I have ACI patches that also bump the version (from 2.69 to 2.70) - Nathaniel's patches gets accepted - I rebase my ACI patches onto master. Git thinks that the 2.69->2.70 change is already done, so it leaves VERSION unchanged. I can solve this locally by telling Git to not merge VERSION automatically, but I think it would be helpful to add a unique comment to each change so that everyone gets a conflict cases like this. Do you agree? -- PetrĀ³ From 064acd3c1ef7524c2525fb9266ff5fe3251d23d3 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 5 Dec 2013 13:31:19 +0100 Subject: [PATCH] Add comment about last change to VERSION When a branch with API version bump is rebased, but the version was also bumped in master, Git thinks the change was already done and loses it. A comment unique to each change will cause a merge conflict in this case, so the developer is reminded to update the number. --- VERSION | 1 + 1 file changed, 1 insertion(+) diff --git a/VERSION b/VERSION index e7d7bc3eab38c57cd9c5b24f13c27a234b9c6f03..694f639d03bf9d02dc577b20358b6018609132d1 100644 --- a/VERSION +++ b/VERSION @@ -90,3 +90,4 @@ IPA_DATA_VERSION=2010061412 IPA_API_VERSION_MAJOR=2 IPA_API_VERSION_MINOR=70 +# Last change: npmccallum - RADIUS support -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel