Re: revised patch for gmime init, with test.

2012-01-14 Thread Pieter Praet
On Fri, 13 Jan 2012 05:05:35 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner  wrote:
> > On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  wrote:
> > > On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  
> > > wrote:
> > 
> > > with differing hashes), this has the potential of causing confusion
> > > and/or quite some extra work when debugging using git-bisect(1), so
> > > I'd like to propose that bugfixes for (to-be-)released code are only
> > > applied on the 'maint' branch ('release' in the case of Notmuch),
> > > and then immediately merged back into 'master'.  In fact, this would
> > > preferrably happen after *every* (series of) commit(s) on the 'maint'
> > > branch, to prevent issues like [1].
> > 
> > There is some merit it to this. On the other hand, it makes the history
> > messier.  [1] would have also been prevented by making the patch against
> > the right branch.
> 
> I thought about this a bit more, and I agree that at least the release
> candidates (basically anything tagged on branch release) ought to be
> merged back to master. Since any series of bugfix patches seems to be
> cause for a new release candidate, this should avoid the need to have
> doubly applied patches.
> 

Thanks!

> I'm less convinced about the need to merge every little doc change and
> debian packaging change back to master right away. This might be a
> purely aesthetic objection; [...]

See my previous reply [1].

> [...] I'm not sure if the extra merge commits
> cause any problems for e.g. bisection.
> 

Infrequent merging increases the possibility of bugs due to unforeseen
interactions between commits on different branches, which is likely to
require one to do a multitude of fake merges (`git merge --no-commit')
in order to properly track down the offending commit, so... frequent
merging would actually *prevent* issues when bisecting.


> d


Peace

-- 
Pieter

[1] id:"878vlar7ka@praet.org"
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: revised patch for gmime init, with test.

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  wrote:
> > On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  wrote:
> 
> > with differing hashes), this has the potential of causing confusion
> > and/or quite some extra work when debugging using git-bisect(1), so
> > I'd like to propose that bugfixes for (to-be-)released code are only
> > applied on the 'maint' branch ('release' in the case of Notmuch),
> > and then immediately merged back into 'master'.  In fact, this would
> > preferrably happen after *every* (series of) commit(s) on the 'maint'
> > branch, to prevent issues like [1].
> 
> There is some merit it to this. On the other hand, it makes the history
> messier.  [...]

This *can* get rather messy when interlacing the 'master'/'maint' merges
with non-rebased changesets pulled from other repos (most often aren't
rebased in advance since they've already been published), which is a
common occurence in the magit [1] repo.

But take Org-mode [2] for example (which is an *extremely* active project),
where non-rebased changesets are rare(ish), and where the 'maint' branch
is constantly being merged back into 'master', resulting in a very clean
ladder-like commit log, eg. :

  --******---**---*---*--- master
 \ / / \
  *---*---*-*---*--- maint
 0.11   bugfix   NEWSbugfix   0.12~rc1


> [...] [1] would have also been prevented by making the patch against
> the right branch.
> 

True, but if we can obviate the need to check if we're on the right
branch altogether, without causing any unwanted side-effects, wouldn't
it be counterproductive not to?


Peace

-- 
Pieter

[1] git://github.com/magit/magit.git
[2] git://orgmode.org/org-mode.git
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: revised patch for gmime init, with test.

2012-01-13 Thread David Bremner
On Fri, 13 Jan 2012 12:52:48 -0800, Jameson Graef Rollins 
 wrote:
> 
> Doesn't everything need to be merged into master eventually anyway?  It
> seems to me that unless it's a change that very narrowly targeting an
> issue in a release branch that is not an issue in master, every patch
> will ultimately need to be applied to both.  It doesn't really make
> sense to me to apply a change to one branch and not the other, if they
> will eventually need to be applied to both anyway.

The following two sequences of commands apply the same changes, but
result in a different history graph.

1) notmuch checkout release && git am patch  && \
   notmuch checkout master && git cherry-pick release

2) notmuch checkout release && git am patch  && \
   notmuch checkout master && git merge release

well, they apply the same changes if release was an ancestor of master
when the they both began.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: revised patch for gmime init, with test.

2012-01-13 Thread Jameson Graef Rollins
On Fri, 13 Jan 2012 05:05:35 -0400, David Bremner  wrote:
> I thought about this a bit more, and I agree that at least the release
> candidates (basically anything tagged on branch release) ought to be
> merged back to master. Since any series of bugfix patches seems to be
> cause for a new release candidate, this should avoid the need to have
> doubly applied patches.
> 
> I'm less convinced about the need to merge every little doc change and
> debian packaging change back to master right away. This might be a
> purely aesthetic objection; I'm not sure if the extra merge commits
> cause any problems for e.g. bisection.

Doesn't everything need to be merged into master eventually anyway?  It
seems to me that unless it's a change that very narrowly targeting an
issue in a release branch that is not an issue in master, every patch
will ultimately need to be applied to both.  It doesn't really make
sense to me to apply a change to one branch and not the other, if they
will eventually need to be applied to both anyway.

jamie.


pgpzHyGS32K3U.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: revised patch for gmime init, with test.

2012-01-13 Thread David Bremner
On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  wrote:
> > On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  wrote:
> 
> > with differing hashes), this has the potential of causing confusion
> > and/or quite some extra work when debugging using git-bisect(1), so
> > I'd like to propose that bugfixes for (to-be-)released code are only
> > applied on the 'maint' branch ('release' in the case of Notmuch),
> > and then immediately merged back into 'master'.  In fact, this would
> > preferrably happen after *every* (series of) commit(s) on the 'maint'
> > branch, to prevent issues like [1].
> 
> There is some merit it to this. On the other hand, it makes the history
> messier.  [1] would have also been prevented by making the patch against
> the right branch.

I thought about this a bit more, and I agree that at least the release
candidates (basically anything tagged on branch release) ought to be
merged back to master. Since any series of bugfix patches seems to be
cause for a new release candidate, this should avoid the need to have
doubly applied patches.

I'm less convinced about the need to merge every little doc change and
debian packaging change back to master right away. This might be a
purely aesthetic objection; I'm not sure if the extra merge commits
cause any problems for e.g. bisection.

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: revised patch for gmime init, with test.

2012-01-12 Thread David Bremner
On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  wrote:
> On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  wrote:

> with differing hashes), this has the potential of causing confusion
> and/or quite some extra work when debugging using git-bisect(1), so
> I'd like to propose that bugfixes for (to-be-)released code are only
> applied on the 'maint' branch ('release' in the case of Notmuch),
> and then immediately merged back into 'master'.  In fact, this would
> preferrably happen after *every* (series of) commit(s) on the 'maint'
> branch, to prevent issues like [1].

There is some merit it to this. On the other hand, it makes the history
messier.  [1] would have also been prevented by making the patch against
the right branch.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: revised patch for gmime init, with test.

2012-01-12 Thread Pieter Praet
On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  wrote:
> It turns out that our existing (trivial) python test is enough to
> catch this bug, but the corpus needs to be augmented.  This
> augmentation is a bit intrusive so I'm thinking of cherry-picking only
> the actual fix to the release branch.
> 

Due to the ~same change being applied in multiple places (and thus
with differing hashes), this has the potential of causing confusion
and/or quite some extra work when debugging using git-bisect(1), so
I'd like to propose that bugfixes for (to-be-)released code are only
applied on the 'maint' branch ('release' in the case of Notmuch),
and then immediately merged back into 'master'.  In fact, this would
preferrably happen after *every* (series of) commit(s) on the 'maint'
branch, to prevent issues like [1].


In this specific case (g_mime_init fix), it would have sufficed to
apply the test suite augmentation patches on 'master' and the bugfix
only on 'maint', merging 'maint' into 'master', and then removing
the "test_subtest_known_broken" line in a followup commit on 'master'.

Thus, maintainers would have fewer merge conflicts to resolve,
developers would at all times benefit from release-specific fixes,
and (pre-)release users would have clear indication of what was fixed
due to the test suite reporting "FIXED" instead of "PASS".


Thanks!

> Unfortunately the test message is 8 bit, so it may be encoded in some
> inconvenient way for patch application. The message is attached to an
> earlier message in the thread if you want to double check.
> 
> I also wondered about putting g_type_init inside the (!initialized)
> test, but decided against it on the grounds of minimality.
> 
> I think we want to in the medium term factor out all of the
> initialization code into one (probably private) function; we can clean
> things up a bit more then.
> 
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter

[1] id:"87mxa95u3d.fsf@zancas.localnet"
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch