Re: [Freeipa-devel] Proposed standard for Patches: RFC

2010-10-27 Thread Adam Young

On 10/26/2010 04:47 PM, Simo Sorce wrote:

On Tue, 26 Oct 2010 16:26:13 -0400
Adam Youngayo...@redhat.com  wrote:

   

I'll admit this would be useful, but it would be another process that
we don't have now, that I was trying to avoid.  We all have git repos
on fedorapeople.  The trick is to deal with patches that have to get
changed prior to commit, hence the numbering of -2 -3 after the seq
number.
 

Not sure what's the problem here, if I rebase a patch you have the
latest one in my tree, no need to look for -1 -2 -3 as you can't be
wrong if you re-fetch from my tree.
   


Yes, and if we go with a Git based appraoch, that would be fine.  But 
the team seems to havea preference and a system in place that works with 
straight patches.  I am not trying to change that.  If you are set on a 
Git based approach, it is a more significant change from our current 
process, something that I would not advocate this close to a major release.


I'm just trying to codify what Rob, Endi and I are already doing, and 
which seems to work well.


   

Really, the seq number is not needed, but makes a nice ready
shorthand for the patch.  Pavel, Endi and I often refer to patches by
number, like your patch 0019  which makes it handy.  The increasing
seq approach to detect a missing packet works in TCP, so why not for
us?
 

Because I am not a machine :)
   


I disagree.  So does schoolhouse rock:
http://www.youtube.com/watch?v=Kdn0pPcTvN4


I see we constantly fail at correctly numbering sequentially stuff,
from new error numbers to OIDs and other stuff, so I do not see this as
a big improvement. I am not saying people shouldn't use this method if
so they prefer, but mandating it seems a bit too much.
   

It seems to be easy enough to do.


Of course if others strongly feel this is the way to go, I will (try to)
comply.
   


Let's give it a go  for a few.


Simo.

   


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] Proposed standard for Patches: RFC

2010-10-27 Thread Adam Young
Made a change based on a recommendation by Simo:  proejct name now 
leads, followed by user name.


Posted on the wiki here:

 http://fedorahosted.org/freeipa/wiki/PatchFormat

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] Proposed standard for Patches: RFC

2010-10-26 Thread Simo Sorce
On Tue, 26 Oct 2010 13:40:11 -0400
Adam Young ayo...@redhat.com wrote:

 We've been doing this informally for a while, and I think, if we all 
 agree to the format, it will help keep track of patches, ACKs, and
 commits.
 
 
 1.  Patch naming
 Example patch name:
 edewata-freeipa-0019-Certificate-management-for-services.patch
 
 
 Format:  username-project-seq[-update]-description.extension
 
 username:  Your Fedora account name.
 project name:   always 'freeeipa'

Are these really necessary ?
We have the name of the author in the patch anyway, and freeipa
(with 2 'e's not 3 :-P) seem really redundant.

Bottomline I am lazy and would prefer not to have to rename patches
after git format-patch creates them.

So I'd like to understand the rationale for this format.
After all we always send patches as attachments to emails, where we can
explain what is going on.

 seq:  sequnece number.  please try to not skip numbers, as we will
 use this number to ensure all patches from a given contributor get
 reviewed. update.  If a patch requires modifications and additional
 prior to submisiion, append a number starting at 2 and increasing by
 one for each update.  Thus, if the above patch required additional
 changes, the first would be:
  edewata-freeipa-0019-2-Certificate-management-for-services.patch
  and then
  edewata-freeipa-0019-3-Certificate-management-for-services.patch
 
 description:  This is the first line of the git commit, and should be 
 less than six words long (idealy two or three).  git format patch
 will translate this line into the subject of the patch, with hyphens 
 replacing the whitespace.
 
 extension:  always .patch
 
 
 2. Patch format:
 All patches should be in format to apply with
   'git am'.
 This is produced from a git repository using the command
 'git format-patch'
 
 
 If a patch addresses a ticket in Trac, the second line of the commit 
 should be the URL to track with the Ticket number.  For example:
 https://fedorahosted.org/freeipa/ticket/339

This part is worth, but I think we should require only the bug number
and have the full URL as a nice optional.

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] Proposed standard for Patches: RFC

2010-10-26 Thread Simo Sorce
On Tue, 26 Oct 2010 14:22:01 -0400
Adam Young ayo...@redhat.com wrote:

 On 10/26/2010 02:08 PM, Simo Sorce wrote:
  On Tue, 26 Oct 2010 13:40:11 -0400
  Adam Youngayo...@redhat.com  wrote:
 
 
  We've been doing this informally for a while, and I think, if we
  all agree to the format, it will help keep track of patches, ACKs,
  and commits.
 
 
  1.  Patch naming
  Example patch name:
  edewata-freeipa-0019-Certificate-management-for-services.patch
 
 
  Format:  username-project-seq[-update]-description.extension
 
  username:  Your Fedora account name.
  project name:   always 'freeeipa'
   
  Are these really necessary ?
  We have the name of the author in the patch anyway, and freeipa
  (with 2 'e's not 3 :-P) seem really redundant.
 
 
 Otherwise, we get into a conflict over who''s patch 519 it is, and we 
 have no way to order it.
 
 We've had enough issues where patch 11 requires patch 10 that it is
 just cleaner to try to apply all patches from a given developer in
 order.

If the problem is tracking which patches have been applied an which are
needed wouldn't it be easier instead if each developer published an
official tree with the patches he proposes for inclusion ?

That way all you need to do is a git log origin/master..dev_tree and
you have all pending patches and the order they are applied to.

Looks to me *much* handier then trying to order them based on file
names and arbitrary sequence numbers.

  If a patch addresses a ticket in Trac, the second line of the
  commit should be the URL to track with the Ticket number.  For
  example: https://fedorahosted.org/freeipa/ticket/339
   
  This part is worth, but I think we should require only the bug
  number and have the full URL as a nice optional.
 
 I just copy and paste from the browser.  It does make it clear
 whether we are talking about Trac or Bugzilla.

bugzilla numbers flies around the 600k mark, looks pretty easy to tell
which is which unless we have a sudden, dramatic spike in tickets
filed against the trac instance :)

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] Proposed standard for Patches: RFC

2010-10-26 Thread Adam Young

On 10/26/2010 03:29 PM, Simo Sorce wrote:

On Tue, 26 Oct 2010 14:22:01 -0400
Adam Youngayo...@redhat.com  wrote:

   

On 10/26/2010 02:08 PM, Simo Sorce wrote:
 

On Tue, 26 Oct 2010 13:40:11 -0400
Adam Youngayo...@redhat.com   wrote:


   

We've been doing this informally for a while, and I think, if we
all agree to the format, it will help keep track of patches, ACKs,
and commits.


1.  Patch naming
Example patch name:
edewata-freeipa-0019-Certificate-management-for-services.patch


Format:  username-project-seq[-update]-description.extension

username:  Your Fedora account name.
project name:   always 'freeeipa'

 

Are these really necessary ?
We have the name of the author in the patch anyway, and freeipa
(with 2 'e's not 3 :-P) seem really redundant.

   

Otherwise, we get into a conflict over who''s patch 519 it is, and we
have no way to order it.

We've had enough issues where patch 11 requires patch 10 that it is
just cleaner to try to apply all patches from a given developer in
order.
 

If the problem is tracking which patches have been applied an which are
needed wouldn't it be easier instead if each developer published an
official tree with the patches he proposes for inclusion ?

That way all you need to do is a git log origin/master..dev_tree and
you have all pending patches and the order they are applied to.

Looks to me *much* handier then trying to order them based on file
names and arbitrary sequence numbers.
   


I'll admit this would be useful, but it would be another process that we 
don't have now, that I was trying to avoid.  We all have git repos on 
fedorapeople.  The trick is to deal with patches that have to get 
changed prior to commit, hence the numbering of -2 -3 after the seq number.


Really, the seq number is not needed, but makes a nice ready shorthand 
for the patch.  Pavel, Endi and I often refer to patches by number, like 
your patch 0019  which makes it handy.  The increasing seq approach to 
detect a missing packet works in TCP, so why not for us?




   

If a patch addresses a ticket in Trac, the second line of the
commit should be the URL to track with the Ticket number.  For
example: https://fedorahosted.org/freeipa/ticket/339

 

This part is worth, but I think we should require only the bug
number and have the full URL as a nice optional.

   

I just copy and paste from the browser.  It does make it clear
whether we are talking about Trac or Bugzilla.
 

bugzilla numbers flies around the 600k mark, looks pretty easy to tell
which is which unless we have a sudden, dramatic spike in tickets
filed against the trac instance :)
   

Yeah, but the full URL approach is self documenting.


Simo.

   


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] Proposed standard for Patches: RFC

2010-10-26 Thread Simo Sorce
On Tue, 26 Oct 2010 16:26:13 -0400
Adam Young ayo...@redhat.com wrote:

 I'll admit this would be useful, but it would be another process that
 we don't have now, that I was trying to avoid.  We all have git repos
 on fedorapeople.  The trick is to deal with patches that have to get 
 changed prior to commit, hence the numbering of -2 -3 after the seq
 number.

Not sure what's the problem here, if I rebase a patch you have the
latest one in my tree, no need to look for -1 -2 -3 as you can't be
wrong if you re-fetch from my tree.

 Really, the seq number is not needed, but makes a nice ready
 shorthand for the patch.  Pavel, Endi and I often refer to patches by
 number, like your patch 0019  which makes it handy.  The increasing
 seq approach to detect a missing packet works in TCP, so why not for
 us?

Because I am not a machine :)
I see we constantly fail at correctly numbering sequentially stuff,
from new error numbers to OIDs and other stuff, so I do not see this as
a big improvement. I am not saying people shouldn't use this method if
so they prefer, but mandating it seems a bit too much.

Of course if others strongly feel this is the way to go, I will (try to)
comply.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel