-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
bpo-33832: Add "magic method" entry to Glossary #7630
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
Conversation
Sorry for being repetitive over other PRs, but could you change the commit messages to include some context or squash them? |
d374a08
to
ea80249
Compare
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.
I think these changes make the doc worse and that this PR should be closed.
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 And if you don't make the requested changes, you will be put in the comfy chair! |
@terryjreedy the initial commit was lost because I was asked to squash all commits, but it's intention was to just add "magic method" to the Glossary pointing to "special method", and add "magic method" as a synonym in Data model. I can revert this changes so you can see the original intention. |
Just for context: I suggested changing the commit messages or squashing because there were two commits with titles "First commit" and "second commit". |
ea80249
to
9cc7887
Compare
PRs best stick to one subject. I would be at least neutral to a simple glossary addition. Add a blurb: Add glossary entry for 'magic method'. Please get in the habit of including a blurb if you think one will be needed. Once present, they are easy to edit through the web interface, without having to pull the PR. |
9cc7887
to
4a191fb
Compare
I agree with marking the term informal. Sorry for not adding the blurb. Didn't know it was needed, and have committed a couple of glossary terms with no blurb being requested, but I'll keep it in mind :) I have made the requested changes; please review again |
Thanks for making the requested changes! @terryjreedy: please review the changes made to this pull request. |
Never squash a PR. The merge message (commit to master repository message) is ultimately written by the core dev who merges. But it is nice if the initial commit message under the title is at least an approximation. It can be edited in, as I just did. The boilerplate message about backporting should be deleted before hitting the [submit PR] button. |
Yes, that was my understanding too, but I was asked to squash the commits. I'll have this in mind next time. |
@terryjreedy Do you approve of the changes made on 4a191fb ? |
I removed the " needs backport to 3.6" label, the 3.6 branch no longer accept bugfixes (only security fixes are accepted): https://devguide.python.org/#status-of-python-branches |
I like this addition! Would one or two index entries also be useful? |
I think so, good idea! I'll add it. |
@merwok Done! I added index entries for magic method and special method as well. |
@merwok perhaps we can merge this? |
Thanks @andresdelfino for the PR, and @merwok for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.7. |
(cherry picked from commit f760610) Co-authored-by: Andre Delfino <[email protected]>
GH-12574 is a backport of this pull request to the 3.7 branch. |
GH-12575 is a backport of this pull request to the 2.7 branch. |
(cherry picked from commit f760610) Co-authored-by: Andre Delfino <[email protected]>
(cherry picked from commit f760610) Co-authored-by: Andre Delfino <[email protected]>
(cherry picked from commit f760610) Co-authored-by: Andre Delfino <[email protected]>
Proposed merge message.
https://bugs.python.org/issue33832