bpo-40492: Fix --outfile when the program being profiled changes the working directory
#19910
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asottile
force-pushed
the
profile_output_file_chdir_40492
branch
from
1c5b71d to
338e784
Compare
taleinat
requested changes
Oct 18, 2020
Misc/NEWS.d/next/Library/2020-05-04-12-16-00.bpo-40492.ONk9Na.rst
Outdated
Show resolved
Hide resolved
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Co-authored-by: Tal Einat <[email protected]>
|
the suggestions seem nitpicky at best, but I've added them -- it seems the devguide has changed at least twice since I've started committing to cpython (I went through my PRs and there's ~3 different styles that were suggested for attribution in them (!)) |
|
I didn't expect the Spanish Inquisition |
|
Nobody expects the Spanish Inquisition! @taleinat: please review the changes made to this pull request. |
Thanks! It is appreciated. The (duplicate) one about the comments is what it is - we have a style guide and we try to stick to it. Yes, it can feel like nitpicking sometimes. I do try to use GitHub's "suggestion" feature to allow applying such suggestions with a click, to make doing so as easy as possible.
That attribution at the end of the NEWS entry wouldn't have needed to be changed by itself, it was fine. The meat of that change request was to clarify what was fixed. I only changed the attribution in my suggestion out of habit. |
just seems like a waste of time after this has been sitting here for 6 months -- I don't really agree with your change to my changelog entry either but given it's already been sitting here without mention for 6 months I simply don't have time to argue about it cpython "adheres to PEP8" in that sometimes it's mentioned in review but the vast majority does not adhere (and certainly not to all of it) -- for instance Lib/profile.py has 8 (!) such comments which don't match your suggestion! I don't run this project but I'd usually just ignore things (or invest in tooling to automatically report / fix them) unless they are actually broken architecturally and even then I'd probably just fix them myself by pushing to the PR 🤷 either way, we're at a place where I think (?) you're happy with the change |
taleinat
approved these changes
Oct 18, 2020
I know what that feels like and I'm sorry for that. Such delays are a big problem, and it's something that I'm personally working to address, as well as several others in the core dev team and in the PSF.
With this I agree, I don't like spending my own time checking code style either, it is not a good investment of time. I intend to bring this up in the upcoming core dev sprint in terms of improving the experience of contributing to the project.
I avoid making such changes directly since for many people that can feel like too direct intervention in their own work. I'll try to remember that you prefer that I just make such changes directly on your PRs. Again, thanks for the PR! 🙏 |
taleinat
added
needs backport to 3.8
needs backport to 3.9
|
Sorry, @asottile and @taleinat, I could not cleanly backport this to |
|
Sorry @asottile and @taleinat, I had trouble checking out the |
asottile
added a commit
to asottile/cpython
that referenced
this pull request
Oct 18, 2020
…it working dir (pythonGH-19910) (cherry picked from commit 3c0ac18)
|
GH-22752 is a backport of this pull request to the 3.9 branch. |
taleinat
pushed a commit
to taleinat/cpython
that referenced
this pull request
Oct 18, 2020
…anges it working dir (pythonGH-19910). (cherry picked from commit 3c0ac18) Co-authored-by: Anthony Sottile <[email protected]>
|
GH-22754 is a backport of this pull request to the 3.8 branch. |
taleinat
pushed a commit
to taleinat/cpython
that referenced
this pull request
Oct 18, 2020
…anges it working dir (pythonGH-19910). (cherry picked from commit 3c0ac18) Co-authored-by: Anthony Sottile <[email protected]>
asottile
added a commit
to asottile/cpython
that referenced
this pull request
Oct 18, 2020
…it working dir (pythonGH-19910) (cherry picked from commit 3c0ac18)
|
adorilson
pushed a commit
to adorilson/cpython
that referenced
this pull request
Mar 13, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://bugs.python.org/issue40492