-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
argparse
docs: normalize constant references
#98765
Conversation
I am happy to respond to comments, but i can no longer push changes. What is going on? |
It might be that you made a change to the git branch using the GitHub website (perhaps by pressing the "Update branch" button), and then forgot to It's hard to diagnose without knowing what the precise error message you're getting is, though ;) |
Thanks @AlexWaygood. Here's what I'm seeing:
As far as I can tell from |
A bare |
If you don't have any local changes you want to keep, there's always the shotgun approach, resetting your local branch to what's on the remote branch: git fetch origin
git reset --hard origin/argparse-doc Then you can commit, push, etc. as before. If you do have any local changes, you can either cherry-pick or rebase them on top. |
I still have no idea how my local copy got weird w.r.t. the github version, but it's back in shape I think. |
I'm still not entirely sure what actually happened on your end without having taken a look myself, but here are my tips to avoid this in the future and save both you time and heartache (and help reviewers too):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small set of suggestions; other than that LGTM
It seems things have gone wrong here again with the commit history due to another local merge—all the commits are duplicated twice. In addition, this is on top of (as was the case originally), the original changes were implemented, manually reverted and redone for unclear reasons, and the rework to remove the const changes, plus the repeated sets of suggestion applications, all of which makes the commit history here very hard to follow. To fix all of this for now, you can just reset your branch to the latest merge with master and apply just the delta in the working tree as a new commit. To do so (tested locally to confirm): git pull
git reset bded5edd9ab # Last commit on main this has been merged with
git commit -am "Doc: Normalize constant references & link sys.stdin/stdout"
git push -f Also, tip for the future: To avoid the time/effort, extra noise and CI resources of applying each suggestion individually, in the |
b61eecb
to
f6c2df8
Compare
I followed your advice on the reset etc stuff. Does it look okay now? I'm not sure what you're referring to when you say "files" tab. I do everything from the command line on my laptop, so always work locally. I'll take a look though and see if i can figure out what you mean. |
Also, the GitHub app (at least on Android) seems to suck... Is it just me? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now @smontanaro , thanks!
Yup, looks great, thanks 👍
I do as well as far as Git is concerned, and also basic GitHub stuff with the Its no big deal if you do of course; I just wanted to mention the tip since a lot of devs aren't aware of that feature but it can save non-trivial time and effort especially with a larger number of suggestions.
I've never used any apps for GitHub myself (either mobile or desktop) other than the |
When you apply review suggestions, as it appears you did for the ones on this
review
<#98765 (review)>,
instead of clicking the Commit button on each suggestion, entering a
message (or leaving the generic one), and clicking Apply, you can instead
go to the Files Changed
<https://github.com/python/cpython/pull/98765/files> tab on the PR, click Add
to batch on each suggestion you want to apply, and then when you've
clicked the button on all of the ones you want, you click the Commit
button at the top or on each one, enter a descriptive message and then
click Apply to commit them all in one go. See the docs
<https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request>
for more details and screenshots.Message ID:
***@***.***>
Ah, thanks. Ignore the email I just sent...
|
Sorry for all the trouble on this one! |
argparse
docs: normalize constant references
Thanks @smontanaro for the PR, and @AlexWaygood for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11. |
Sorry, @smontanaro and @AlexWaygood, I could not cleanly backport this to |
GH-98807 is a backport of this pull request to the 3.11 branch. |
(cherry picked from commit b27b57c) Co-authored-by: Skip Montanaro <[email protected]>
GH-98808 is a backport of this pull request to the 3.10 branch. |
(cherry picked from commit b27b57c)
(cherry picked from commit b27b57c) Co-authored-by: Skip Montanaro <[email protected]>
Thanks, Skip! |
Second attempt. In the future, please give me the opportunity to respond to comments before summarily closing a PR. It just seems rude to me.