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

bpo-35641: IDLE - calltip not properly formatted if no docstring #11415

Merged
merged 6 commits into from
Jan 3, 2019

Conversation

eamanu
Copy link
Contributor

@eamanu eamanu commented Jan 2, 2019

bpo-35641: IDLE: calltips not properly formatted for functions without doc-strings

https://bugs.python.org/issue35641

@eamanu eamanu requested a review from terryjreedy as a code owner January 2, 2019 19:43
@eamanu eamanu changed the title Fix bpo-35641: IDLE: calltips not properly formatted for functions without doc-strings bpo-35641: IDLE: calltips not properly formatted for functions without doc-strings Jan 2, 2019
@terryjreedy terryjreedy self-assigned this Jan 2, 2019
@terryjreedy terryjreedy changed the title bpo-35641: IDLE: calltips not properly formatted for functions without doc-strings bpo-35641: IDLE - calltip not properly formatted if no docstring Jan 2, 2019
Copy link
Contributor

@taleinat taleinat left a comment

Choose a reason for hiding this comment

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

Other than my inline comment, two more things:

  1. Add a test for this bug. It should fail without the fix and pass with the fix.
  2. Add a NEWS entry. The easiest way to do so is to follow the "Details" link next to the failing "bedevere/news" check near the bottom of this page, which will lead you to the "blurb-it" tool.

@bedevere-bot
Copy link

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Copy link
Contributor

@remilapeyre remilapeyre left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

When you write a commit message, make it more descriptive about what the commit does, it's very important when diving into git log, here are some tips about commit messages: https://devguide.python.org/pullrequest/#making-good-commits

You need to use blurb add or create manually an entry to Misc/NEWS.

Please add some test to your pull request :)

remove debug line
@eamanu
Copy link
Contributor Author

eamanu commented Jan 2, 2019

Thanks for your comments. I will push tests

Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

What tal said. Add at least one test and a blurb file. I will likely edit whatever you write before merging, so don't agonize over the wording. But with a file, I can do so in the online pencil editor.

@terryjreedy terryjreedy added needs backport to 3.7 type-bug An unexpected behavior, bug, or error labels Jan 2, 2019
@terryjreedy
Copy link
Member

Tal, since our change requests were identical, I will merge this when CI passes.

@terryjreedy terryjreedy merged commit ab54b9a into python:master Jan 3, 2019
@miss-islington
Copy link
Contributor

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

@bedevere-bot
Copy link

GH-11420 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 3, 2019
…-11415)

(cherry picked from commit ab54b9a)

Co-authored-by: Emmanuel Arias <emmanuelarias30@gmail.com>
miss-islington added a commit that referenced this pull request Jan 3, 2019
(cherry picked from commit ab54b9a)

Co-authored-by: Emmanuel Arias <emmanuelarias30@gmail.com>
@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

Sorry, @eamanu and @terryjreedy, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker ab54b9a130c88f708077c2ef6c4963b632c132b3 3.7

@terryjreedy terryjreedy added type-bug An unexpected behavior, bug, or error needs backport to 3.7 and removed needs backport to 3.7 type-bug An unexpected behavior, bug, or error labels Jan 3, 2019
@miss-islington
Copy link
Contributor

Thanks @eamanu for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

Sorry, @eamanu and @terryjreedy, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker ab54b9a130c88f708077c2ef6c4963b632c132b3 3.7

@taleinat
Copy link
Contributor

taleinat commented Jan 3, 2019

Sorry for the noise; both Terry and I added the "needs backport to 3.7", which caused the second backport attempt to fail.

@eamanu eamanu deleted the fix35641 branch January 3, 2019 12:11
@terryjreedy
Copy link
Member

Better than neither of us ;-). Part of the problem was that the robots + GitHub were really slow in posting anything, so I removed and re-added the label after the backport has been done.

@terryjreedy
Copy link
Member

I just noticed that the blurb was put in the Library directory instead of the Idle directory. #11479 moves it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants