Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-134861: Add CSV output format to asyncio ps#134862
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
dpdani commented May 28, 2025 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
# Conflicts: # Lib/asyncio/tools.py
ZeroIntensity commented May 28, 2025
(Skipping news because this is an easter egg) |
johnzhou721 commented May 28, 2025
@ZeroIntensity Hmm... But isn't the CSV format legit, so in my (very bad) opinion it still need news? |
ZeroIntensity commented May 28, 2025
Oh, hm. I guess we could add an entry for only CSV. I'll leave the decision to @dpdani. |
Uh oh!
There was an error while loading. Please reload this page.
AA-Turner commented May 29, 2025
Yes, please add news for CSV format |
johnzhou721 left a comment
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.
Yep, agreed with the NEWS part. But might it make the Easter Egg too obvious? Since people can just look at the PR through news. I think we might be able to split up the PRs into CSV and BSV and skip news on the latter, but it might be too much work.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
AA-Turner commented May 29, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
We're not trying to hide or obsfucate; just not to advertise. |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
python-cla-botbot commented May 29, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
johnzhou721 commented May 29, 2025 via email
Sure, resolve. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
dpdani commented Aug 1, 2025
I think there's not much point duplicating the issue now, but splitting into two PRs can make sense. @picnixz if you really think it's best, I can do the split. |
picnixz commented Aug 1, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Yeah no need for two issues, but I would really like two separate PRs. For people reading commits, it's easier to spot the easter egg and it also makes reverting easier (if needed). It also allows us to make a funnier commit message. However, if @1st1 or any other codeowner is ok with adding the easter egg in this PR, I won't oppose their decision. EDIT: I pinged the wrong code owner |
dpdani commented Aug 6, 2025
I'll split the PR now |
asyncio psasyncio psdpdani commented Aug 6, 2025
Split done. The CSV format is here and the news entry is already ok. |
Uh oh!
There was an error while loading. Please reload this page.
AA-Turner commented Aug 6, 2025
As far as I can tell there are no existing tests for this, #133284 has some for |
dpdani commented Aug 6, 2025
Yeah, I saw it didn't have tests before and didn't think it would be necessary to add them here. What do you think? |
dpdani commented Aug 6, 2025
Ah ok, you enabled auto-merge, I guess it's ok 👍 Thank you! |
470cbe9 into python:mainUh oh!
There was an error while loading. Please reload this page.
…ython#134862) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
… ps`` (python#134862)" This reverts commit 470cbe9.
asyncio ps#134861