bpo-33832: Add "magic method" entry to Glossary by andresdelfino · Pull Request #7630 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation24 Commits2 Checks0 Files changed

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

[ Show hidden characters]({{ revealButtonHref }})

andresdelfino changed the title~~bpo-33832: Make "magic methods" a little more discoverable in the docs~~ bpo-33832: Make magic methods a little more discoverable in the docs

Jun 11, 2018

Sorry for being repetitive over other PRs, but could you change the commit messages to include some context or squash them?

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 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.

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".

PRs best stick to one subject. I would be at least neutral to a simple glossary addition.
I don't want to promote the term 'magic method', but it is in use and I suspect various core developers are partly to blame. So beginners do run across it and wonder how 'magic methods' are different from 'special methods'. I would be even happier with 'an informal synonym ...'.

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.

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.

andresdelfino changed the title~~bpo-33832: Make magic methods a little more discoverable in the docs~~ bpo-33832: Add "magic method" entry to Glossary

Jun 18, 2018