On 7 October 2016 at 11:55, Chris Angelico wrote:

If you want to, you can 'git cherry-pick -x 142a57', which will add a linking comment to the commit that it creates, saying where it came from.

Recommending the use of referenced cherry picks sounds like a good idea to me. It may also be worth linking to https://www.python.org/dev/peps/pep-0512/#commit-multi-release-changes-in-bu... from this section of the devguide to help explain why we recommend using the "work on master, cherry pick to maintenance branches" approach. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On 7 October 2016 at 12:28, Chris Angelico wrote:

git checkout master git cherry-pick footnote git show

Check out the branch you want to cherry-pick into. $ git checkout 3.6 Grab the commit you want $ git cherry-pick -x 142a57 Make sure it looks good, and then push.

That note about "make sure it looks good" reminded me that "make patchcheck" is going to need updating to handle git, which means updating Tools/scripts/patchcheck.py Brett, how would you like that extra task captured - PR to PEP 512? Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On 7 October 2016 at 13:26, Zachary Ware wrote:

On Thu, Oct 6, 2016 at 10:10 PM, Nick Coghlan wrote:

...

That note about "make sure it looks good" reminded me that "make patchcheck" is going to need updating to handle git, which means updating Tools/scripts/patchcheck.py

Brett, how would you like that extra task captured - PR to PEP 512?

I just checked, `make patchcheck` works fine from a Git checkout.

Could, if it basically works already, we probably don't need to note it anywhere. My comment was based on the fact that there are references to both Mercurial and Mercurial Queues in the patchcheck source code, but I didn't look in detail at what they actually do. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On Sat, Oct 8, 2016 at 8:31 AM, Maciej Szulik wrote:

There's one nit with that cherry-pick, it already creates the commit, and current description states you should create a commit after running tests [1].

[1] http://cpython-devguide.readthedocs.io/en/github/committing.html#merging-bet...

Not sure that that need be a problem. You've already run the tests on the other branch, presumably, so the chances of a test failure are low; most of the time, it'll be "cherry-pick, test, push", and very occasionally, it'll be "cherry-pick, test, oops failures, reset or amend". As long as you don't push, there's nothing permanent in the commit. ChrisA

On Sat, Oct 8, 2016 at 9:14 AM, Maciej Szulik wrote:

Totally agree, that's why I went with that change and left the history behind. But run tests is still required :)

Oh sure. Definitely run the tests. No disagreement there :) ChrisA

On 8 October 2016 at 13:27, Barry Warsaw wrote:

On Oct 07, 2016, at 11:31 PM, Maciej Szulik wrote:

...

There's one nit with that cherry-pick, it already creates the commit, and current description states you should create a commit after running tests [1].

You can also use cherry-pick -n/--no-commit.

It probably makes more sense to assume a successful backport as the default case, and cover "git commit --amend" to handle the case where the tests fail - the ease with which history can be rewritten prior to publication is the main UX benefit git offers over hg, so we may as well take advantage of it. That is, there are 3 reasonably common possible flows: - backport with no merge conflicts, tests pass (default flow) - backport with merge conflicts (git will prompt for this) - backport with test failures (cover with "git commit --amend") Recommending "--no-commit" as the default makes the first flow longer by adding a separate commit step without actually simplifying the others. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On Sun, Oct 9, 2016 at 11:20 PM, Barry Warsaw wrote:

On Oct 09, 2016, at 12:43 PM, Nick Coghlan wrote:

...

It probably makes more sense to assume a successful backport as the default case

We'll have to see. Maybe Python's code won't change so much between major versions, but I've been on projects where refactorings and other changes do result in most cherry picks needing massage.

In my experience, CPython is stable enough that you can sometimes even pick up a patch from *2.7* and apply it to the latest 3.x. (The downside of this is that some files are oddly named, eg longobject.c, which implements what we now call "int".) Between 3.5 and 3.6 it'll be even more common for a patch to apply cleanly, and shortly after a branch is cut (eg current 3.6 -> 3.7), near-certain. And of the possible failure modes, "patch doesn't apply" is far more likely than "patch applies but tests fail". In the former case, git will tell you straight away; it's only in the latter case that you'll need to amend the commit.

...

, and cover "git commit --amend" to handle the case where the tests fail - the ease with which history can be rewritten prior to publication is the main UX benefit git offers over hg, so we may as well take advantage of it.

Yeah; I have a visceral reaction to history modification, but git does seem to be more amenable to that. I can't check now, but what's the failure mode if you --amend a pushed commit? IIRC, the --amend succeeds but you'll get an error on push, which IMO is later than it should be (i.e. git probably should disallow --amend on published commits).

It's pretty simple: rosuav@sikorsky:~/Gypsum$ git push To github.com:Rosuav/Gypsum ! [rejected] master -> master (non-fast-forward) error: failed to push some refs to 'git@github.com:Rosuav/Gypsum' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. (And attempting a 'git pull' will highlight the fact that you've edited history.) The server will want to be configured to utterly reject "non-fast-forward pushes"; git actually does allow you to override the above message ("git push --force-with-lease" and friends), but ultimately, it's the central server that chooses to accept or reject the push. It's possible, for instance, to accept a force-push on a topic branch but reject them on your primary branches, which would allow people to rebase their "proposed change" branches freely (eg to update them to the current codebase) without permitting any damage to the version branches "3.5", "3.6", "master", etc. In GitHub terms, this is done by protecting a branch: https://help.github.com/articles/about-protected-branches/ In a more directly-managed environment, this would be implemented with a pre-receive or update hook: https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks As this is a simple script, it has all the flexibility of code, so any rule can be implemented.

...

Recommending "--no-commit" as the default makes the first flow longer by adding a separate commit step without actually simplifying the others.

That's true, but I generally always want to review what git does before I publish it, and not committing makes that easier for me.

The other thing is that there are lots of ways to accomplish things in git, and everyone will have their favorite workflow. That's totally fine as long as things stay local. It's okay to be opinionated and document the expected common good path, but most important is to document anything that will have global effects, because that affects everyone. Maybe think about using words like MUST and SHOULD to delineate the difference (a la RFCs).

That would be a personal workflow choice, then. +1 on RFC 2119 MUST vs SHOULD usage. IMO the document should specify *one* recommended workflow, because otherwise it's just way too confusing to read; and then by its language, it can clarify which parts you're allowed to diverge from and which not. Most of git _does_ stay local, and to the greatest extent possible, the server should be set up to accept or reject commits that don't comply with the workflow's MUSTs. If that central server is under python.org's direct control, that simply means codifying all the MUSTs into the pre-receive/update hook (see above), the code for which can then be published alongside the workflow documents as a sort of "clarifying document". ChrisA

On Mon, Oct 10, 2016 at 10:34 PM, Nick Coghlan wrote:

On 10 October 2016 at 01:12, Chris Angelico wrote:

...

In my experience, CPython is stable enough that you can sometimes even pick up a patch from *2.7* and apply it to the latest 3.x. (The downside of this is that some files are oddly named, eg longobject.c, which implements what we now call "int".) Between 3.5 and 3.6 it'll be even more common for a patch to apply cleanly, and shortly after a branch is cut (eg current 3.6 -> 3.7), near-certain.

Right, it's mainly when you collide with a refactoring that things get painful (e.g. the SSL/TLS rebase in Python 2.7 ran into that).

Makes sense. So it would probably be worth cherry-picking a patch one branch at a time; I would expect that such code changes, once done, are reflected in all future branches, so you'd have something like "3.3-3.4 work this way, and 3.5-3.7 work that way". Should be pretty obvious once people get started on it.

In general though, we shouldn't see any more problems with cherry picks than we see with forward merges, and there Misc/NEWS is the only consistent source of conflicts.

Is Misc/NEWS needed for old branches? If not, the easiest way might be to simply build the patch without NEWS, then cherry-pick it to all other branches, and finally add NEWS to the latest branch only.

...

That would be a personal workflow choice, then. +1 on RFC 2119 MUST vs SHOULD usage. IMO the document should specify *one* recommended workflow, because otherwise it's just way too confusing to read; and then by its language, it can clarify which parts you're allowed to diverge from and which not.

A lot of the SHOULDs will be "Use the web-based workflow to review and merge changes, since that offloads more of the work to automated systems and can be done from any client device".

While there will be cases where local client usage is needed (and generating a backport PR will be one of them, unless/until GitHub copies GitLab's cherry-pick-in-the-web-UI feature), allowing reviews to be done that way is mainly for the benefit of current core devs that prefer to do their reviews locally.

Sounds good to me. Those who are comfortable with local tools are welcome to use them; if you've become as familiar with gitk as I have, you won't want to give it up. (hgk/'hg view' is a poor clone of gitk, with a fraction of its power. Don't judge gitk by hgk!) The default workflow will be the nice simple web-based one, but there are plenty of equivalents. GitHub will notice that you've pushed all the commits in a PR and automatically mark it as "Merged", for instance, so you don't have to worry about going and doing some bookkeeping. ChrisA