GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-29271: Fix Task.current_task and Task.all_tasks to accept None. by 1st1 · Pull Request #406 · python/cpython

Conversation

@1st1

@1st1 1st1 commented Mar 2, 2017

Copy link
Copy Markdown
Member

See http://bugs.python.org/issue29271 for context. LGTM'ed by @methane

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Yury Selivanov
bpo-29271: Fix Task.current_task and Task.all_tasks to accept None.
ed0e527
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Mar 2, 2017
@mention-bot

mention-bot commented Mar 2, 2017

Copy link
Copy Markdown

@1st1, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Haypo, @methane and @serhiy-storchaka to be potential reviewers.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka reviewed Mar 2, 2017
View reviewed changes
Comment thread Misc/NEWS Outdated
- Issue #24142: Reading a corrupt config file left configparser in an
invalid state. Original patch by Florian Höch.

- Issue #29271: Fix Task.current_task and Task.all_tasks to accept

@serhiy-storchaka serhiy-storchaka Mar 2, 2017

Copy link
Copy Markdown
Member

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.

Move the entry to the start of the section. Mention that this fixes C implementation, makes it conforming to Python implementation.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@1st1 1st1 Mar 2, 2017

Copy link
Copy Markdown
Member Author

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.

Aren't we usually appending news items instead of prepending?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_asyncio/test_tasks.py Outdated
# See http://bugs.python.org/issue29271 for details:
asyncio.set_event_loop(loop)
try:
self.assertTrue(Task.current_task(None) is task)

@serhiy-storchaka serhiy-storchaka Mar 2, 2017

Copy link
Copy Markdown
Member

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.

Why not use assertIs()?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@1st1 1st1 Mar 2, 2017

Copy link
Copy Markdown
Member Author

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.

good catch

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Yury Selivanov
Address Serhiy's feedback.
e9cb8a4
serhiy-storchaka
serhiy-storchaka reviewed Mar 2, 2017
View reviewed changes
Comment thread Misc/NEWS Outdated
Library
-------

- Issue #29271: Fix Task.current_task and Task.all_tasks implemented in C

@serhiy-storchaka serhiy-storchaka Mar 2, 2017

Copy link
Copy Markdown
Member

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.

Now Misc/NEWS entries should be prefixed by "bpo-XXXXX" rather of "Issue #XXXXX".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@serhiy-storchaka

serhiy-storchaka commented Mar 2, 2017 via email

Copy link
Copy Markdown
Member
Aren't we usually appending news items instead of prepending?
Entries in Misc/NEWS should be prepended. Entries in What's New usually are appended.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Yury Selivanov
Fix Misc/NEWS entry
3cdab39
@1st1

1st1 commented Mar 2, 2017

Copy link
Copy Markdown
Member Author

Thank you for helping out Serhiy. It's my first PRs to the new GH repo so I'm getting accustomed to some new practices.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka approved these changes Mar 2, 2017
View reviewed changes
@1st1 1st1 added cherry-pick for 3.6 and removed cherry-pick for 3.6 labels Mar 2, 2017
@1st1

1st1 commented Mar 2, 2017

Copy link
Copy Markdown
Member Author

@serhiy-storchaka BTW, to backport this to 3.6, do I need to create a new PR? Or I can just cherry-pick and push?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented Mar 2, 2017 via email

Copy link
Copy Markdown
Member
AFAIK you can't push directly into this repository. You need to create a new PR. Add the label "need to backport to 3.6" to this PR and "cherry-pick for 3.6" to the new PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@1st1
Merge branch 'master' into bpo-29271
39bd564
@1st1
1st1 merged commit 8d26aa9 into python:master Mar 3, 2017
@1st1 1st1 mentioned this pull request Mar 3, 2017
bpo-29271: Fix Task.current_task and Task.all_tasks to accept None. #413
Merged
1st1 added a commit to 1st1/cpython that referenced this pull request Mar 3, 2017
@1st1 Yury Selivanov
bpo-29271: Fix Task.current_task and Task.all_tasks to accept None. (p…
13802a3 
…ython#406)
@berkerpeksag berkerpeksag removed the needs backport to 3.6 label Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@1st1 @mention-bot @serhiy-storchaka @berkerpeksag @the-knights-who-say-ni
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.