Re: [Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION

2013-12-09 Thread Tomas Babej

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

Re: [Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION

2013-12-09 Thread Martin Kosek
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

2013-12-09 Thread Simo Sorce
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

2013-12-09 Thread Petr Viktorin

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

[Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION

2013-12-05 Thread Petr Viktorin

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 pvikt...@redhat.com
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