Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use f-string in preference to "...".format() #98883

Merged
merged 5 commits into from Nov 3, 2022

Conversation

smontanaro
Copy link
Contributor

While this does modernize the argparse tutorial example code in a trivial way, I'm doing this mostly to see if I can cleanly generate a PR which doesn't get all messed up in the face of various changes (see below).

For those reading along, I think my error was that I created my working branch from origin/main (smontanaro/cpython, which was up-to-date w.r.t. python/cpython at the time of the branch, and updated periodically) instead of following this instruction in Lifecycle of a Pull Request and branching from upstream/main:

git checkout -b <branch-name> upstream/main

So, if you have been dragged here by my references (@AlexWaygood, @JelleZijlstra, @CAM-Gerlach), I would appreciate your feedback on my presumed error.

No matter whether this seems worth accepting or not (I don't really care), please leave this open for a day or two so I can continue messing around with things to see if I can break it again (hopefully not).

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is worth changing; it's good to point people to current best practices in tutorials.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Oct 31, 2022

Git Investigative Team accident report on the PR #98765 incident

The Git Investigative Team has conducted a detailed analysis of the PR #98765 incident, and has reconstructed the sequence of events, probable cause/contributing factors and safety recommendations from the available information.

Executive Summary

The probable cause of the PR #98765 accident was committing remote and local changes without a git pull in between, followed by rebasing the local branch on main instead of the remote branch, an then merging between these two duplicate branches. Recommendations that would have avoided the accident include:

  • Taking advantage of GitHub suggestions for straightforward changes, rather than manually re-implementing them locally
  • Avoiding updating remote branches on GitHub if not necessary (such as to resolve a merge conflict, apply a specific set of new commits, or test code with the latest changes).
  • Always applying suggestions and git pull before making local changes on an outstanding PR branch

Timeline of Events

Note: Readers can follow along and safety explore the final state of the accident branch themselves using git checkout b61eecb, with ``git log --graph` displaying a useful visual reconstruction. Due to the events of the accident and subsequent force push destroying the original branch, the following sequence has been reconstructed from archived logs and empirical modeling.

Accident Branch History

  1. The accident branch arparse-doc was created from main branch commit 3c4cbd1 at some point between 10:19 UTC October 15, the time of that commit, and 16:27 UTC Oct 22, when the first commit was made to it. It did not include commit 05c042e7 to main at 13:30 UTC Oct 15, or any following it. The author stated that their local main branch was up to date with that of upstream at the time they created the branch, so if this was the case, they must have created it between 10:19 and 13:30 UTC October 15, and only committed to it 7 days later. While they have stated this was not strictly following the relevant SOP, even if the branch had in fact been out of date at the time of first commit, this was ultimately not a significant factor in the accident.
  2. Before the original PR was opened, between Oct 22 and 21:45 UTC on Oct 26, 16ffb2d, 7037311 and 33b5dec were committed. making the original changes later proposed in the PR. Their minimal descriptions made it difficult for investigators to ascertain their exact purpose and motivation, but this did not directly contribute to the accident.
  3. Still before the original PR was opened, at 00:06:01 UTC on Oct 27, commit 6cd3700 with message "back to main version" reverted all the changes previously committed to the branch, for unknown reasons. Investigators could not determine whether this was done via git revert, git checkout main Doc/library/argparse.rst, or another method. More concerning, we could not determine a plausible reason for this, nor why this was done as a commit rather than simply resetting the the branch pointer back to the current main (via git reset main), as it had occurred prior to the PR being opened. However, while this likely caused confusion and distraction for reviewers, and perhaps the author, it did not appear to directly contribute to the primary accident.
  4. Two minutes and 15 seconds later, at 00:08:16 UTC, 3694f11 was committed, which re-implements the same alterations, only omitting a few tweaks unrelated to the core changes. Given the short time between the two changes, it seems unlikely, though not impossible, that everything was implemented by hand, and rather more probable that the previous changes were retained in the working tree. However, there was again no clear apparent reason for this approach.
  5. At 00:11 UTC on Oct 27, the original PR, Use :const: and :data: as appropriate #98750 , was opened with these changes, and no further changes were made to it.
  6. Reviewers requested substantial changes on the PR, but at 05:24 UTC on Oct 27, it was unilaterally closed by another reviewer, resulting in the author later opening a new PR on the same branch rather than waiting for it to be re-opened. While neither action directly led to the subsequent accident, avoiding either may have reduced any confusion surrounding it.
  7. At 11:55 UTC Oct 27, ff85d94 was committed to address the reviewer feedback.

Accident Sequence

  1. At 12:00:08 UTC, PR argparse docs: normalize constant references #98765 was opened on branch argparse-doc, as it existed on the @smontanaro remote.
  2. 4 seconds later, at 12:00:12 UTC, someone (presumably the PR author) pressed the Update branch button on PR argparse docs: normalize constant references #98765 , which pushed merge commit dc5796e, updating the remote branch to main commit 22863df. The investigators could not determine the exact reason for doing so, but it was at this point that the local and the remote branch diverged.
  3. In response to an immediate reviewer suggestion, bd420b0 (after later rebase) was manually committed to the author's local branch at 12:03 UTC , without either pulling the now-updated remote branch, or simply applying the suggestion directly to it. This left both the local and remote branch with unique commits, meaning a merge or rebase would be necessary to resolve the situation.
  4. By 12:06 UTC, the author had attempted to push this new commit to the remote branch but could not, due to the merge with main update triggered on the remote branch, and reported the problem on the PR. This resulted in the reviewer committing the suggestion to the remote branch themselves, as 69615b9 , at 12:07 UTC.
  5. At 12:16 UTC, the author makes another commit, e9c25f9 (after later rebase) to their local branch to address another review comment, but again cannot push it, because their local branch is still out of date.
  6. Nearly simultaneously, the reviewer correctly infers and reports the cause of the problem, that a change was made on GitHub to the remote branch that was not pulled locally before changes were made and attempted to be pushed, which the author's error message at 12:40 UTC confirms. At 13:21 UTC the reviewer responds with remedial actions, running git pull, that should have fixed it by merging the remote changes locally (though possibly with the need to resolve merge conflicts, and a less straightforward commit history). However, it is unclear whether this was ever attempted as instructed.
  7. Later, at 18:18:55 UTC, the PR author merges the remote branch again with the latest main, this time to commit 9d8214b with merge commit 35133db, likely also with the GitHub web interface. This has little overall effect, either positive or negative, on the situation, aside from potential distraction.
  8. A mere 20 seconds later, however, a far more serious and damaging event occurs, At 18:19:15 UTC, the local branch is rebased against that very same main commit, starting with the first commit 9d8214b. As this rewrites the commit history from the first unique commit in the branch, there are now two entirely distinct and nearly (but not quite) duplicate sets of commits locally and remotely. At this point, recovery with git pull is no longer possible, as it will produce a merged branch with two duplicate sets of commits.
  9. At 18:21 UTC, another reviewer mentions that if necessary, the author can hard-reset their local branch to that of the remote, though any local commits will be lost and need to be cherry-picked or redone. Even at this point, this still would have resolved the immediate problem, albeit with the loss of the second change in response to review comments.
  10. However, at 18:26 UTC, the author pulls the remote branch into their local one, with merge commit f30ca39 , and at 18:28 UTC reports they believe they have resolved the problem. At this point, they will indeed be able to push, but their commit history now has two duplicate sets of commits.
  11. In response, at 18:42 UTC a reviewer, while speculating as to the cause, suggests three future prevention strategies, any of which would indeed have avoided the current problem. At 18:49 UTC, they follow up with two review suggestions on the PR.
  12. At some point between 18:42 and 19:13 UTC, the now duplicated local branch is pushed to the remote branch. At this point, the aforementioned "reset local branch to remote" response will no longer work, at least without modification (resetting to a specific commit, and force-pushing).

Aftermath and cleanup

  1. At 19:14 UTC, the two review suggestions are applied, individually as suggestions, on the PR to the remote branch.
  2. At 20:37 UTC, a reviewer notices the problem and recommends a suitable response to resolve all the previous issues cleanly, while liquidating the now nearly irreparable commit history: soft-resetting the local branch to main, committing the delta from the working tree as a single commit, and force-pushing.
  3. At 00:05 UTC on October 28, the author executes the requested steps and pushes the result, resolving the problems. The PR is then merged in that state.

Probable cause/contributing factors

The probable cause of the PR #98765 accident was committing remote and local changes without a git pull in between, followed by rebasing the local branch on main instead of the remote branch, an then merging between these two duplicate branches.

Directly contributing were:

  • Manual local application of remote GitHub suggestions
  • Unnecessary updating of the remote branch
  • Difficulty in diagnosing a complex and largely unknown local problem

Indirect contributing factors include:

  • Submitting a PR from a branch with an outdated main
  • Unnecessary revert and re-doing in the commit history prior to publication
  • The premature closure of the original PR, and the unnecessary opening of a new PR
  • Lack of clear, descriptive commit messages
  • Use of a merge rather than a rebase for local changes not yet pushed to the remote

Recommendations for authors

Preventative measures

  • Take advantage of GitHub suggestions for straightforward changes, rather than manually re-implementing them locally
  • Conversely, avoid updating remote branches on GitHub if not necessary (such as to resolve a merge conflict, apply a specific set of new commits, or test code with the latest changes).
  • If making local changes is necessary, always apply suggestions and git pull before doing so on an outstanding PR branch

Response actions

If a git push is rejected, either:

  • git pull to merge in the remote changes, albeit at the cost of greater commit complexity, or
  • git fetch && git rebase origin/<feature-branch-name> to rebase on the remote changes, keeping a linear history (while still not rewriting history for others) at the cost of slightly more effort

Also, in this situation:

  • Never force push your local changes, or the remote changes will be lost
  • Don't rebase locally on main, as this will cause the problem seen here while not fixing anything
  • Use git log --graph to display the commit graph, and always verify it shows what you expect before pushing

Additional recommendations

  • Use sufficiently descriptive commit messages to clearly explain the purpose and motivation of each commit to others
  • If a PR has not been opened, ensure your commit history is cleaned up as needed to make sense, and avoid cluttering the commit log with needless revert and redo commits, git reset main and re-commit, or use git rebase -i main can be used to fix things up
  • Before pushing a branch and opening a public PR, hub sync/git pull/etc. the latest main and git rebase main your feature branch.
  • Avoid making a new PR if a PR already exists for the same branch; if closed, simply re-open it or ask for it to be re-opened, if needed

Appendix: Full commit graph

Expand for full commit graph

$ git logg --stat
* commit b61eecb1f974647ce7f87d3d8c17545d3018d001 (HEAD -> argparse-doc)
| Author: Skip Montanaro <skip.montanaro@gmail.com>
| Date:   Thu Oct 27 14:14:20 2022 -0500
|
|     Update Doc/library/argparse.rst
|
|     Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
|
|  Doc/library/argparse.rst | 4 ++--
|  1 file changed, 2 insertions(+), 2 deletions(-)
|
* commit 75ae9e9a88ee28c6dbfa7b0c350a8d21a816abbc
| Author: Skip Montanaro <skip.montanaro@gmail.com>
| Date:   Thu Oct 27 14:13:59 2022 -0500
|
|     Update Doc/library/argparse.rst
|
|     Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
|
|  Doc/library/argparse.rst | 2 +-
|  1 file changed, 1 insertion(+), 1 deletion(-)
|
*   commit f30ca393b1931be1bf2597f9e2ab40f357c56c0a
|\  Merge: e9c25f9384f 35133db0945
| | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | Date:   Thu Oct 27 13:26:13 2022 -0500
| |
| |     Merge remote-tracking branch 'remotes/origin/argparse-doc' into argparse-doc
| |
| *   commit 35133db0945ffee2561405073784d9f7680d4387
| |\  Merge: 69615b9f6a1 bded5edd9ab
| | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | Date:   Thu Oct 27 13:18:55 2022 -0500
| | |
| | |     Merge branch 'python:main' into argparse-doc
| | |
| * | commit 69615b9f6a1b0540b8d61948aa757693261d9fa5
| | | Author: Alex Waygood <Alex.Waygood@Gmail.com>
| | | Date:   Thu Oct 27 13:07:31 2022 +0100
| | |
| | |     Fix reST syntax typo
| | |
| | |  Doc/library/argparse.rst | 2 +-
| | |  1 file changed, 1 insertion(+), 1 deletion(-)
| | |
| * |   commit dc5796e0113352dd21538b9bacc0124c5507bf47
| |\ \  Merge: ff85d949a3d 22863df7ca5
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Thu Oct 27 07:00:17 2022 -0500
| | | |
| | | |     Merge branch 'main' into argparse-doc
| | | |
| * | | commit ff85d949a3dd8228b07d915cdf94275c6a8fc90b
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Thu Oct 27 06:55:08 2022 -0500
| | | |
| | | |     correct markup to the preferred style (code without link generation)
| | | |
| | | |  .../argparse.rst       | 64 +++---
| | | |  1 file changed, 32 insertions(+), 32 deletions(-)
| | | |
| * | | commit 3694f11901acfe2d2e45e82f3ce241874ff52511
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Wed Oct 26 19:08:16 2022 -0500
| | | |
| | | |     use :const: & :data:
| | | |
| | | |  .../argparse.rst       | 68 +++---
| | | |  1 file changed, 34 insertions(+), 34 deletions(-)
| | | |
| * | | commit 6cd3700dfafb632e46dfffc59b0d53b39a340569
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Wed Oct 26 19:06:01 2022 -0500
| | | |
| | | |     back to main version
| | | |
| | | |  .../argparse.rst       | 72 +++---
| | | |  1 file changed, 36 insertions(+), 36 deletions(-)
| | | |
| * | | commit 33b5dec709762c65d9f7e9e918ef25a64bc4c853
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Wed Oct 26 16:45:00 2022 -0500
| | | |
| | | |     tag sys.stdin/stdout as data
| | | |
| | | |  Doc/library/argparse.rst | 4 ++--
| | | |  1 file changed, 2 insertions(+), 2 deletions(-)
| | | |
| * | | commit 7037311f460e704ed1ce296afdcd73b7c72dbd79
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Tue Oct 25 08:59:07 2022 -0500
| | | |
| | | |     qualify
| | | |
| | | |  Doc/library/argparse.rst | 2 +-
| | | |  1 file changed, 1 insertion(+), 1 deletion(-)
| | | |
| * | | commit 16ffb2d278c7cdf5c2a2f9312118cbe57ddfb78e
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Sat Oct 22 11:27:24 2022 -0500
| | | |
| | | |     minor changes
| | | |
| | | |  .../argparse.rst       | 66 +++---
| | | |  1 file changed, 33 insertions(+), 33 deletions(-)
| | | |
* | | | commit e9c25f9384f359e33d42aeeb8ecca76b5a614954
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Thu Oct 27 07:16:53 2022 -0500
| | | |
| | | |     respond to Alex's comment
| | | |
| | | |  .../argparse.rst       | 5 +++--
| | | |  1 file changed, 3 insertions(+), 2 deletions(-)
| | | |
* | | | commit bd420b0216338a93c4690c778200709754514883
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Thu Oct 27 07:03:14 2022 -0500
| | | |
| | | |     key bounce? ;-)
| | | |
| | | |  Doc/library/argparse.rst | 2 +-
| | | |  1 file changed, 1 insertion(+), 1 deletion(-)
| | | |
* | | | commit 52c606ca9d59352ea46513ea683931b9c1e58e95
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Thu Oct 27 06:55:08 2022 -0500
| | | |
| | | |     correct markup to the preferred style (code without link generation)
| | | |
| | | |  .../argparse.rst     | 64 +++---
| | | |  1 file changed, 32 insertions(+), 32 deletions(-)
| | | |
* | | | commit b3e733eac67fcca669510dc6b86d571b1dc3ba35
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Wed Oct 26 19:08:16 2022 -0500
| | | |
| | | |     use :const: & :data:
| | | |
| | | |  .../argparse.rst     | 68 +++---
| | | |  1 file changed, 34 insertions(+), 34 deletions(-)
| | | |
* | | | commit 8f4fe6a3de9a377712af60d1845d5603ca449a11
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Wed Oct 26 19:06:01 2022 -0500
| | | |
| | | |     back to main version
| | | |
| | | |  .../argparse.rst     | 72 +++---
| | | |  1 file changed, 36 insertions(+), 36 deletions(-)
| | | |
* | | | commit a0fc7434577acadef0185996ab58b99f4bc29f19
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Wed Oct 26 16:45:00 2022 -0500
| | | |
| | | |     tag sys.stdin/stdout as data
| | | |
| | | |  .../argparse.rst        | 4 ++--
| | | |  1 file changed, 2 insertions(+), 2 deletions(-)
| | | |
* | | | commit 5f121f54673987179603bbf439fd4257339d8747
| | | | Author: Skip Montanaro <skip.montanaro@gmail.com>
| | | | Date:   Tue Oct 25 08:59:07 2022 -0500
| | | |
| | | |     qualify
| | | |
| | | |  Doc/library/argparse.rst | 2 +-
| | | |  1 file changed, 1 insertion(+), 1 deletion(-)
| | | |
* | | | commit 9d8214b901349023255b4092fb23205d218888fb
| |_|/  Author: Skip Montanaro <skip.montanaro@gmail.com>
|/| |   Date:   Sat Oct 22 11:27:24 2022 -0500
| | |
| | |       minor changes
| | |
| | |    Doc/library/argparse.rst | 66 ++++++------
| | |    1 file changed, 33 insertions(+), 33 deletions(-)
| | |
* | | commit bded5edd9abf7ae6b2874916d70ec29ad209217c (upstream/main, whatsnew, main)
| | | Author: Benjamin Peterson <benjamin@python.org>
| | | Date:   Thu Oct 27 09:06:49 2022 -0700
| | |
| | |     obmalloc: Remove unused variable. (GH-98770)
| | |
* | | commit 723ebe76e787cfa6b08cc9587dd679f3234a1025
| |/  Author: Erlend E. Aasland <erlend.aasland@protonmail.com>
|/|   Date:   Thu Oct 27 15:06:48 2022 +0200
| |
| |       gh-96143: Improve perf profiler docs (#96445)
| |
* | commit 22863df7ca5f9cd01a40ab3dce3d067ec5666081
| | Author: Mark Shannon <mark@hotpy.org>
| | Date:   Thu Oct 27 03:55:03 2022 -0700
| |
| |     GH-96793: Change `FOR_ITER` to not pop the iterator on exhaustion. (GH-96801)
| |

≈≈≈    Many commits

| |
* | commit 05c042e70786bd2e3fcb274d185e1e0a54dab5a7
| | Author: Joannah Nanjekye <33177550+nanjekyejoannah@users.noreply.github.com>
| | Date:   Sat Oct 15 09:30:05 2022 -0400
| |
| |     gh-85525: Indicate supported sound header formats (#21575)
| |     Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
| |
* | commit 3c4cbd177f36777a04e78eb07ce20367560a66d3
|/  Author: Julien Palard <julien@palard.fr>
|   Date:   Sat Oct 15 12:19:35 2022 +0200
|
|       gh-91485: Doc: Using Python syntax to document builtin Python functions. (GH-96579)
|
* commit 2fe44f728afa2dd506c304641f0481d6813d1dbd
| Author: Steve Dower <steve.dower@python.org>
| Date:   Fri Oct 14 16:58:54 2022 +0100
|
|     gh-98251: Allow venv to pass along PYTHON* variables to pip and ensurepip when they do not impact path resolution (GH-98259)
|

@CAM-Gerlach
Copy link
Member

For those reading along, I think my error was that I created my working branch from origin/main (smontanaro/cpython, which was up-to-date w.r.t. python/cpython at the time of the branch, and updated periodically) instead of following this instruction in Lifecycle of a Pull Request and branching from upstream/main:

The branch you're creating your new branch from only matters isofar as what commit it happens to point to, such that the first commit on that branch will have that commit as its parent. After you create the new branch, it has no connection to the branch (or tag, or commit hash, or other ref) you created it off of, other than the commit that ref pointed to at the moment you created it will be the parent of the first commit in the branch. If both origin/main and upstream/main pointed to the same commit at the time you created them, it makes no difference which you had used.

More details

You can think of commit in a branch as items in one-directional linked lists (they're actually more like trees, but let's keep things simple), each with some content and the static immutable hash of the parent commit. Branches are just mutable pointers to particular HEAD commit, and so you can think of them as linked lists of commits. The branch pointer gets "dereferenced" to the specific static commit hash when creating a new branch, so it doesn't matter what branch pointer you use, just what commit hash it was pointing to.

Therefore, this is unrelated to your problem. The one potential area for problems is if you'd committed directly to your fork's main branch and submitted a PR with that, e.g. like this one python/peps#2854 , but you created a new branch just fine. The problems happened after that.

No matter whether this seems worth accepting or not (I don't really care), please leave this open for a day or two so I can continue messing around with things to see if I can break it again (hopefully not).

FYI, I added DO-NOT-MERGE, but you should also consider marking the PR as draft (see the right sidebar; its a small link that's easy to miss), which actually makes it not mergable (DO-NO-MERGE is merely advisory, nothing actually prevents it from being merged), and also makes it more clear to reviewers that it is in a draft state and should not be closed summarily.

@smontanaro smontanaro marked this pull request as draft October 31, 2022 13:23
@smontanaro
Copy link
Contributor Author

@CAM-Gerlach thanks for the detailed explanation. I will accept it as the gospel truth with the exception of this:

Response actions
If a git push is rejected, either:

  • git pull to merge in the remote changes, albeit at the cost of greater commit complexity, or
  • git fetch && git rebase origin/ to rebase on the remote changes, keeping a linear history (while still not rewriting history for others) at the cost of slightly more effort

When the rejection first appeared, the very first thing I did was the best of my ability to follow the git pull ... suggestion. Every single time I tried this (I did it multiple times over the course of my experience) I was told everything was up-to-date. I clearly messed things up, but that was only after following Git's suggested remedy. By the time I asked for help I was clearly flailing and had totally messed things up. No doubt I should have asked for help sooner, but I didn't. So be it. Whether I will ever gain proficiency with Git or not is another question. I may be back with more tales of woe. :-(

Thanks for the "make draft" suggestion as well. I have done that and will proceed to try updating the branch....

@smontanaro
Copy link
Contributor Author

Here's my next dilemma. I updated using the above button, then in my checkout (I happened to be on my argparse-doc branch), I executed git fetch origin, then git pull. That generated a sizable dribble of output about files being updated and created. So far, so good.

I then switched to my argparse-howto branch (the one for this PR) and executed git pull again. Git reported:

There is no tracking information for the current branch.
Please specify which branch you want to rebase against.
See git-pull(1) for details.

    git pull <remote> <branch>

If you wish to set tracking information for this branch you can do so with:

    git branch --set-upstream-to=<remote>/<branch> argparse-howto

Looking back at the Lifecycle document it seems reasonable to suggest that users also execute git branch --set-upstream-to=origin/<branch> <branch>. It seems likely that in the common case a PR will undergo revision and such tracking will need to be set up anyway. Might as well state it explicitly ("explicit is better than implicit" ya know...)

@smontanaro
Copy link
Contributor Author

Perhaps of use to this crowd, I opened a bug report for the devguide:

python/devguide#976

@merwok
Copy link
Member

merwok commented Oct 31, 2022

When the rejection first appeared, the very first thing I did was the best of my ability to follow the git pull ... suggestion. Every single time I tried this (I did it multiple times over the course of my experience) I was told everything was up-to-date.

This might be because your local remote-tracking branch for origin/main was synchronized with the main branch in your fork, but that branch was not itself synchronized with the main branch in the upstream CPython repo.

I use git fetch --all to get all branches updated from all remotes (origin = my fork, upstream = python-dev repo)

@CAM-Gerlach
Copy link
Member

When the rejection first appeared, the very first thing I did was the best of my ability to follow the git pull ... suggestion. Every single time I tried this (I did it multiple times over the course of my experience) I was told everything was up-to-date.

If your local feature branch was out of date with the branch you were pushing to, but it was up to date from the branch you were pulling from, then the branch you were pulling from vs. pushing to must have been different branches. My guess would be that you were trying to pull from your fork (orgin's) main branch (which hadn't been been updated to the latest upstream main, and thus was still up to date with your feature branch), rather than from your feature branch.

I updated using the above button, then in my checkout (I happened to be on my argparse-doc branch), I executed git fetch origin, then git pull. That generated a sizable dribble of output about files being updated and created. So far, so good.

So, it would seem your other branch is tracking some branch, but it may not be the one you want. To confirm this was just an issue with your setup on that branch and not your remote/branch configuration, you could send the following output and I can double check.

git remote -v
git remote show origin
git remote show upstream
git branch -vv

Looking back at the Lifecycle document it seems reasonable to suggest that users also execute git branch --set-upstream-to=origin/ .

Instead of all that, you can simply specify -u when pushing for the first time, e.g. git push -u origin argparse-howto, and then using simply git pull and git push that will push to the tracked branch on the remote (and avoid accidentally pulling from, e.g., origin/main as seems might have happened here). In fact, Git's own help output recommends this when you run a bare git push. I'm not sure why the devguide doesn't just recommend that, as that's the standard git workflow AFAIK, but the commands they show are correct, they explicitly specify the remote and branch on every pull and push.

@smontanaro
Copy link
Contributor Author

I appreciate all the assistance trying to figure out what I might or might not have done to get myself into such a mess. @CAM-Gerlach I really hope there was some hefty automation behind that investigation and that it wasn't all human generated. It really wasn't necessary, but thanks just the same. I was happy to discover my initial mistake and strive to do better next time. It reminds me a bit of a forensic pathologist on a police drama using ambient temperature, cloud cover, wind speed and direction, and the type and location of maggots on a rotting corpse to determine time of death... :-)

@smontanaro smontanaro marked this pull request as ready for review November 1, 2022 03:50
@smontanaro
Copy link
Contributor Author

I took this out of draft mode. Whenever y'all are ready, feel free to merge and close. Thanks again for your help.

@CAM-Gerlach
Copy link
Member

Unfortunately, I'm not that smart and had way too much "fun" with it, heh. My notional idea for the framing was a nuclear reactor accident, although the actual structure, content, tone and phrasing were much more heavily inspired by NTSB aircraft accident reports, which I read way too much of in my spare time.

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @smontanaro

@JelleZijlstra JelleZijlstra self-assigned this Nov 2, 2022
@JelleZijlstra JelleZijlstra merged commit 1fd20d0 into python:main Nov 3, 2022
@miss-islington
Copy link
Contributor

Thanks @smontanaro for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @smontanaro and @JelleZijlstra, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 1fd20d0b57478d8b0d8d58718fa773135348bf98 3.11

@bedevere-bot
Copy link

GH-99033 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Nov 3, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 3, 2022
…H-98883)

(cherry picked from commit 1fd20d0)

Co-authored-by: Skip Montanaro <skip.montanaro@gmail.com>
@JelleZijlstra JelleZijlstra added needs backport to 3.11 only security fixes and removed needs backport to 3.11 only security fixes labels Nov 3, 2022
@miss-islington
Copy link
Contributor

Thanks @smontanaro for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-99034 is a backport of this pull request to the 3.11 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 3, 2022
…H-98883)

(cherry picked from commit 1fd20d0)

Co-authored-by: Skip Montanaro <skip.montanaro@gmail.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Nov 3, 2022
miss-islington added a commit that referenced this pull request Nov 3, 2022
(cherry picked from commit 1fd20d0)

Co-authored-by: Skip Montanaro <skip.montanaro@gmail.com>
miss-islington added a commit that referenced this pull request Nov 3, 2022
(cherry picked from commit 1fd20d0)

Co-authored-by: Skip Montanaro <skip.montanaro@gmail.com>
@smontanaro smontanaro deleted the argparse-howto branch February 4, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants