-
-
Notifications
You must be signed in to change notification settings - Fork 844
Use :cpy-file:
throughout the Devguide
#984
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
@@ -17,7 +17,7 @@ while any other issues can and should be decided by any committer. | |||
|
|||
Developers can choose to follow labels, so if a label that they are | |||
following is added to an issue or pull request, they will be notified | |||
automatically. The :file:`CODEOWNERS` file is also used to indicate | |||
automatically. The :cpy-file:`.github/CODEOWNERS` file is also used to indicate |
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.
If we add ~
support to :cpy-file:
we could hide the dir.
@@ -101,12 +101,12 @@ C API Tests | |||
Tests for the public C API live in the ``_testcapi`` module. | |||
Functions named ``test_*`` are used as tests directly. | |||
Tests that need Python code (or are just easier to partially write in Python) | |||
live in ``Lib/test``, mainly in :file:`Lib/test/test_capi.py`. | |||
live in ``Lib/test``, mainly in :cpy-file:`Lib/test/test_capi`. |
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.
test_capi
is now a dir.
@@ -142,7 +143,7 @@ In general, unless you are working on the critical core of the compiler, memory | |||
management can be completely ignored. But if you are working at either the | |||
very beginning of the compiler or the end, you need to care about how the arena | |||
works. All code relating to the arena is in either | |||
:file:`Include/Internal/pycore_pyarena.h` or :file:`Python/pyarena.c`. | |||
:cpy-file:`Include/internal/pycore_pyarena.h` or :cpy-file:`Python/pyarena.c`. |
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.
internal
is lowercase. There were a few similar errors in this file.
@@ -532,12 +535,12 @@ Important Files | |||
|
|||
asdl_c.py |
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.
All the files in this section should be linkified. I left a comment about it in the 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.
Gave a few a spot check, looks good.
@@ -101,12 +101,12 @@ C API Tests | |||
Tests for the public C API live in the ``_testcapi`` module. | |||
Functions named ``test_*`` are used as tests directly. | |||
Tests that need Python code (or are just easier to partially write in Python) | |||
live in ``Lib/test``, mainly in :file:`Lib/test/test_capi.py`. | |||
live in ``Lib/test``, mainly in :cpy-file:`Lib/test/test_capi`. |
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.
We could also link things that aren't already :file:
, like Lib/test
:
live in ``Lib/test``, mainly in :cpy-file:`Lib/test/test_capi`. | |
live in :cpy-file:`Lib/test`, mainly in :cpy-file:`Lib/test/test_capi`. |
There's a lot more that could be linked on this page too, especially the first few sections.
But let's keep the scope of this PR to replacing the old :file:
with the new :cpy-file:
.
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.
Agreed on keeping the PR focused. We should also try to create links only when/where they are useful, but there are definitely more places where :cpy-file:
can be used.
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.
That SGTM
This PR replaces the use of
:file:
with:cpy-file:
(introduced in #961) in most places. Since the generated links are now checked bymake linkcheck
, I also went through and corrected some of those.There are a few broken links that I haven't touched yet:
Include/Python-ast.h
was removed by @vstinnerInclude/token.h
was removed by @vstinnerInclude/code.h
was removed by @vstinnerPython/peephole.c
was removed by @markshannonThe respective sections in
developer-workflow/grammar.rst
andinternals/compiler.rst
might need to be revisited.@vstinner and @markshannon: can you advise on whether these are quick fixes that we can include in this PR or if they should be handled separately?
I also found another issue with this section: https://devguide.python.org/internals/compiler/#important-files
In addition to the fact that the listed files don't use any markup, the markup used for most of the files seem incorrect (it's a blockquote due to the indentation). A nested list with full paths for all the files and
:cpy-file:
should be a better alternative.The
:cpy-file:
role could also be improved to support~
, so that in that list (and in a few other places) we can hide the full path and just display the file name.