-
-
Notifications
You must be signed in to change notification settings - Fork 925
Document attr and attribute #1311
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
|
First, of all, please could you sign the CLA? |
|
Can you mention the Issue number on the PR description (instead of in the title?) E.g. |
Done! Thank you. |
Done!! |
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.
Thanks!
Let's compare "functions definitions" and "functions references" in the quick reference table:
Let's add "attribute definitions" and "attribute references" to the table. The attribute definition are already in the "Information units" section, and let's add the references to the "Roles" section.
[fix] commit suggestion Co-authored-by: Hugo van Kemenade <[email protected]>
[fix] delete dup Co-authored-by: Hugo van Kemenade <[email protected]>
|
@python/organization-owners Please could you enable https://pre-commit.ci for this repo? |
done! |
documentation/markup.rst
Outdated
| True/False/None ````True````, ````False````, ````None```` :ref:`inline-markup` | ||
| functions definitions ``.. function:: print(*args)`` :ref:`directives` | ||
| functions references ``:func:`print``` :ref:`roles` | ||
| attribute definitions ``.. attribute: `attr-name``` :ref:`directives` |
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.
| attribute definitions ``.. attribute: `attr-name``` :ref:`directives` | |
| attribute definitions ``.. attribute: `attr-name``` :ref:`information-units` |
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'm not sure this should be added. In the table I used .. function:/:func: as an example, but the same applies for classes, methods, attributes and several other directives/roles pairs.
Perhaps a separate table associating directives with the roles that can be used to link to them would be better? (If this is done, it should be a separate PR.).
documentation/markup.rst
Outdated
|
|
||
| Refer to an attribute, use the ``:attr:`` role:: |
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.
| Refer to an attribute, use the ``:attr:`` role:: | |
| Refer to an attribute using the ``:attr:`` role:: |
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.
While this is useful here, the same could be done for other directives too, like the class just above or the method just below. It would be nice if the documentation was consistent.
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.
While this is useful here, the same could be done for other directives too, like the
classjust above or themethodjust below. It would be nice if the documentation was consistent.
should it be seperate pr?
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.
Either way is fine, but smaller PRs are easier to review and merge.
Would you like to make a separate follow-up PR? :)
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.
Either way is fine, but smaller PRs are easier to review and merge.
Would you like to make a separate follow-up PR? :)
sure. and when will this pr be merged ? #1311
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.
Let's do it now :)
Thank you for the PR!
information-units Co-authored-by: Hugo van Kemenade <[email protected]>
, using Co-authored-by: Hugo van Kemenade <[email protected]>
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.
Thank you!

Document attr and attribute. Closes #1308
📚 Documentation preview 📚: https://cpython-devguide--1311.org.readthedocs.build/documentation/markup/