matplotlib-devel Mailing List for matplotlib

On 14 October 2012 21:22, Eric Firing <ef...@ha...> wrote:
> 3) The potential disagreement is over whether the PEP8 changes should be
> cherry-picked into v1.2.x, or simply left in master. I favor the latter
> course.
I'm not familiar with matplotlib's merge strategy, but I'd agree with
you that making those changes post-RC doesn't sound sensible. There's
always the chance of something getting broken, especially when diffs
get too big to review carefully.
Thomas

On 2012年10月14日 12:44 PM, Damon McDougall wrote:
> On Sun, Oct 14, 2012 at 9:22 PM, Eric Firing <ef...@ha...> wrote:
>> All,
>>
>> I think we are in a messy situation, and we need to reach some agreement
>> as to how to proceed. This has been discussed a bit in this thread:
>>
>> http://sourceforge.net/mailarchive/forum.php?thread_name=507AFDC6.8000801%40hawaii.edu&forum_name=matplotlib-devel
>>
>> The name of that thread did not reflect the importance of the discussion
>> it prompted, hence the present message.
>>
>> To summarize my view:
>>
>> 1) We have a flood of PEP8 PRs based on master, many of which have been
>> merged, some by myself--so I have no objection to this aspect of the
>> situation, though I would have preferred a slower pace, a garden hose
>> rather than a fire hose. I am happy to see continued merging of these
>> PRs into master.
>>
>> 2) We are also trying to stabilize v1.2.x, getting in the last few bug
>> fixes and doc updates, so we can get a release out, with a high
>> probability that it will be solid.
>>
>> 3) The potential disagreement is over whether the PEP8 changes should be
>> cherry-picked into v1.2.x, or simply left in master. I favor the latter
>> course. First, because massive code churn shortly before a release
>> seems unwise. Second, because I think we should stick to the strategy
>> we started with, in which an effort is made to choose the most
>> appropriate target for each PR, frequently merge the maintenance branch
>> into master, and reserve cherry-picking for occasional corrections.
>>
>> 4) The PEP8 changes will cause some merge problems no matter what we do;
>> but I think that they can be minimal and manageable if we leave PEP8 out
>> of v1.2.x, and decide that once it is released, v1.2.x will be changed
>> only if critical bugs are found, requiring a v1.2.1 release. This also
>> assumes that we have only a few changes left to be made in v1.2.x before
>> a final rc and a release.
>>
>> Therefore I recommend that the PEP8 changes that have already been
>> cherry-picked into v1.2.x be removed from v1.2.x, and that the v1.2.x
>> milestone be removed from all PEP8 changes.
>>
>> If some of the PEP8 commits include genuine bug-fixes that need to be in
>> v1.2.x, then these fixes should be made via PRs directly against v1.2.x.
>>
>> Agreement? Disagreement? Discussion? Related aspects of strategy?
>>
>> Eric
>
> I'm happy with whatever is decided. I'd rather not have merge
> conflicts, but if PEP8 is seen as a high-risk merge then I'm happy to
> not cherry-pick them into 1.2.x.
>
> If it is decided that we are to revert all the PEP8 changes in 1.2.x,
> what should be done about PEP8 changes that were merged into master
> before the v1.2.x branch was created?
>
I didn't know there were any; but anything that far back should be left 
alone, because subsequent things are based on it, and it has been 
getting tested along the way during the rc cycle. My concern is with 
massive changes shortly before release, and about getting the release 
over and done with so we can move on.
Eric

On Sun, Oct 14, 2012 at 9:22 PM, Eric Firing <ef...@ha...> wrote:
> All,
>
> I think we are in a messy situation, and we need to reach some agreement
> as to how to proceed. This has been discussed a bit in this thread:
>
> http://sourceforge.net/mailarchive/forum.php?thread_name=507AFDC6.8000801%40hawaii.edu&forum_name=matplotlib-devel
>
> The name of that thread did not reflect the importance of the discussion
> it prompted, hence the present message.
>
> To summarize my view:
>
> 1) We have a flood of PEP8 PRs based on master, many of which have been
> merged, some by myself--so I have no objection to this aspect of the
> situation, though I would have preferred a slower pace, a garden hose
> rather than a fire hose. I am happy to see continued merging of these
> PRs into master.
>
> 2) We are also trying to stabilize v1.2.x, getting in the last few bug
> fixes and doc updates, so we can get a release out, with a high
> probability that it will be solid.
>
> 3) The potential disagreement is over whether the PEP8 changes should be
> cherry-picked into v1.2.x, or simply left in master. I favor the latter
> course. First, because massive code churn shortly before a release
> seems unwise. Second, because I think we should stick to the strategy
> we started with, in which an effort is made to choose the most
> appropriate target for each PR, frequently merge the maintenance branch
> into master, and reserve cherry-picking for occasional corrections.
>
> 4) The PEP8 changes will cause some merge problems no matter what we do;
> but I think that they can be minimal and manageable if we leave PEP8 out
> of v1.2.x, and decide that once it is released, v1.2.x will be changed
> only if critical bugs are found, requiring a v1.2.1 release. This also
> assumes that we have only a few changes left to be made in v1.2.x before
> a final rc and a release.
>
> Therefore I recommend that the PEP8 changes that have already been
> cherry-picked into v1.2.x be removed from v1.2.x, and that the v1.2.x
> milestone be removed from all PEP8 changes.
>
> If some of the PEP8 commits include genuine bug-fixes that need to be in
> v1.2.x, then these fixes should be made via PRs directly against v1.2.x.
>
> Agreement? Disagreement? Discussion? Related aspects of strategy?
>
> Eric
I'm happy with whatever is decided. I'd rather not have merge
conflicts, but if PEP8 is seen as a high-risk merge then I'm happy to
not cherry-pick them into 1.2.x.
If it is decided that we are to revert all the PEP8 changes in 1.2.x,
what should be done about PEP8 changes that were merged into master
before the v1.2.x branch was created?
-- 
Damon McDougall
http://www.damon-is-a-geek.com
B2.39
Mathematics Institute
University of Warwick
Coventry
West Midlands
CV4 7AL
United Kingdom

All,
I think we are in a messy situation, and we need to reach some agreement 
as to how to proceed. This has been discussed a bit in this thread:
http://sourceforge.net/mailarchive/forum.php?thread_name=507AFDC6.8000801%40hawaii.edu&forum_name=matplotlib-devel
The name of that thread did not reflect the importance of the discussion 
it prompted, hence the present message.
To summarize my view:
1) We have a flood of PEP8 PRs based on master, many of which have been 
merged, some by myself--so I have no objection to this aspect of the 
situation, though I would have preferred a slower pace, a garden hose 
rather than a fire hose. I am happy to see continued merging of these 
PRs into master.
2) We are also trying to stabilize v1.2.x, getting in the last few bug 
fixes and doc updates, so we can get a release out, with a high 
probability that it will be solid.
3) The potential disagreement is over whether the PEP8 changes should be 
cherry-picked into v1.2.x, or simply left in master. I favor the latter 
course. First, because massive code churn shortly before a release 
seems unwise. Second, because I think we should stick to the strategy 
we started with, in which an effort is made to choose the most 
appropriate target for each PR, frequently merge the maintenance branch 
into master, and reserve cherry-picking for occasional corrections.
4) The PEP8 changes will cause some merge problems no matter what we do; 
but I think that they can be minimal and manageable if we leave PEP8 out 
of v1.2.x, and decide that once it is released, v1.2.x will be changed 
only if critical bugs are found, requiring a v1.2.1 release. This also 
assumes that we have only a few changes left to be made in v1.2.x before 
a final rc and a release.
Therefore I recommend that the PEP8 changes that have already been 
cherry-picked into v1.2.x be removed from v1.2.x, and that the v1.2.x 
milestone be removed from all PEP8 changes.
If some of the PEP8 commits include genuine bug-fixes that need to be in 
v1.2.x, then these fixes should be made via PRs directly against v1.2.x.
Agreement? Disagreement? Discussion? Related aspects of strategy?
Eric

On 2012年10月14日 12:17 AM, Damon McDougall wrote:
> On Sun, Oct 14, 2012 at 2:23 AM, Eric Firing <ef...@ha...> wrote:
>> That would be my preference; but has Mike written anything about where PEP8
>> changes should go?
>
> All I can find is this: https://github.com/matplotlib/matplotlib/pull/1153
>
Thanks. It's better than nothing, but hardly crystal-clear as guidance 
for the present situation.
Mike's idea, as a compromise, was to "put some cleanup things (such as 
this) prior to the RC" but after creation of the branch. It is now well 
past rc3. It seems to me that putting such massive changes in now means 
that we should get them all in, then have another RC, and then wait a 
while.
In addition, if this is to be the course of action, I think it would be 
better to rebase each PR against v1.2.x prior to review and merging so 
that we can be inspecting exactly the changes that will be made to 
v1.2.x, instead of cherry-picking. I did ask Mike earlier about 
cherry-picking backwards versus merging maintenance into master, and he 
confirmed that the latter remained the normal practice.
My first choice would still be to not put any of the PEP8 things in 
1.2.x, and absolutely minimize future changes on 1.2.x, freezing it ASAP.
Eric

On Sun, Oct 14, 2012 at 2:23 AM, Eric Firing <ef...@ha...> wrote:
> That would be my preference; but has Mike written anything about where PEP8
> changes should go?
All I can find is this: https://github.com/matplotlib/matplotlib/pull/1153
-- 
Damon McDougall
http://www.damon-is-a-geek.com
B2.39
Mathematics Institute
University of Warwick
Coventry
West Midlands
CV4 7AL
United Kingdom

On 2012年10月13日 1:52 PM, Damon McDougall wrote:
> On Sun, Oct 14, 2012 at 1:29 AM, Eric Firing <ef...@ha...> wrote:
>> On 2012年10月13日 1:16 PM, Damon McDougall wrote:
>>> I probably should have tested the waters first, but I added a PEP8
>>> github label. It's neon orange so you can't miss it. The reason I did
>>> this is so that the list of v1.2.x milestoned issues can be easily
>>> filtered by eye. That way it looks less daunting (since PEP8 isn't a
>>> huge priority for version 1.2 (at least not compared to bug fixes))
>>> and it's easier to see the more important issues.
>>>
>>> It's also temporary. Nelle's done a great job trawling through the
>>> codebase and raising some important points. There's a finite amount of
>>> bulk work to do and then the label can be removed.
>>>
>>> If anyone is offended by neon orange, please feel free to change it. I
>>> just wanted to be able to organise things a little more succinctly.
>>>
>>
>> I don't care about the color, but I don't understand the rationale for
>> putting these PEP8 changes in v1.2.x, especially when they are based on
>> master. It seemed to me that the thing to do was get out a v1.2 release
>> without the PEP8 changes, and let them be the basis in master for
>> proceeding to v1.3.
>>
>> Evidently I was wrong, and we are instead doing massive cherry-picking
>> from master.
>>
>> A disadvantage of this is that PEP8 changes, though seemingly innocuous,
>> could introduce subtle bugs, so rushing them in at the end of the rc
>> cycle seems like it is taking an unnecessary risk for no functional benefit.
>>
>> Eric
>
> The downside of merging them into master without cherry-picking into
> 1.2 is the horrific merge conflicts that will occur whenever there's a
> pull request based on 1.2.
To clarify, you are referring to what would happen if we did not 
cherry-pick, when bug-fixes are made in 1.2, and then 1.2 is merged into 
master? My preference would be to minimize this problem by moving 
quickly on 1.3, with *very* few changes in 1.2.x after the release--just 
quick fixes of critical bugs, if any, for a quick followup release, if 
necessary. In that case there are very few changes that need to be 
merged from 1.2.x into master, hence minimal merge conflicts.
Part of my puzzlement is why, if the strategy is to get all the PEP8 
changes into 1.2, Nelle hasn't been asked to base them on 1.2.x, so they 
could be merged from there into master in the usual way, by merging in 
the whole branch. It seems to me that cherry-picking from master into 
1.2 should be something one does rarely, to fix an error of judgment as 
to which branch a change should go to, not something that should be done 
routinely with a whole series of PRs.
>
> I see your point of introducing subtle bugs. I'm happy to wait on the
> PEP8 changes if it seems too risky.
>
That would be my preference; but has Mike written anything about where 
PEP8 changes should go?
Eric