Skip to content

Improved the writing documentation page. #18714

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

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

pauloxnet
Copy link
Member

Trac ticket number

"N/A"

Branch description

Improved the writing documentation page.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

Copy link
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

Changes looking good, a few small things to suggest. I think you should explain the rationale for the change in how commands are displayed? Seems a bit arbitrary (unless this is part of a docs styleguide I’m not aware of)

``_build/linkcheck/output.txt`` and ``_build/linkcheck/output.json``.

.. admonition:: Be careful with the time
Copy link
Member

Choose a reason for hiding this comment

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

I like having that note but the heading isn’t super clear to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have suggestions for a clearer title?

Copy link
Contributor

Choose a reason for hiding this comment

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

This can be a warning without a title

Copy link
Member Author

Choose a reason for hiding this comment

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

Make sense to me. I removed the title, and updated the commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

This can be a warning without a title

@sarahboyce I just find out that the admonition directive require a title :)

...writing-documentation.txt:196: ERROR: Error in "admonition" directive:
1 argument(s) required, 0 supplied.

So I added a tittle again in the code because the use of the note directive is discouraged in the same documents.

To improve readability, use .. admonition:: Descriptive title rather tha .. note::

See https://docs.djangoproject.com/en/stable/internals/contributing/writing-documentation/#guidelines-for-restructuredtext-files

I like having that note but the heading isn’t super clear to me.

@thibaudcolas I tried to add a clearer title

Copy link
Contributor

@sarahboyce sarahboyce Oct 25, 2024

Choose a reason for hiding this comment

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

I think warnings can have no title (.. warning::)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think warnings can have no title (.. warning::)

Using .. warning:: seems a good idea, thanks, I checked that it's widely used in the code. I just updated the PR. Thanks.

@pauloxnet
Copy link
Member Author

... I think you should explain the rationale for the change in how commands are displayed?

TL;DR I simply applied the presentation format of the 'make html' command to the other commands presented on the page.

Trying to read the guide to contributing to Django documentation from scratch as if I were a new contributor, I noticed that the command to generate the html documentation was very obvious, clear and compatible even for Windows users. The other commands presented on the page, however, were immersed in the text, not easy to locate and with the only variant for Unix users.

@pauloxnet pauloxnet force-pushed the docs/writing branch 2 times, most recently from b4d5f24 to e4a96bc Compare October 24, 2024 07:56
Copy link
Member

@cliff688 cliff688 left a comment

Choose a reason for hiding this comment

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

Could you also link to <coding-style-pre-commit>) in line 309.

@pauloxnet
Copy link
Member Author

Could you also link to <coding-style-pre-commit> in line 309.

Thanks, I did it.

Copy link
Contributor

@sarahboyce sarahboyce left a comment

Choose a reason for hiding this comment

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

Thank you @pauloxnet ⭐ these are nice updates

@sarahboyce sarahboyce merged commit b5669f0 into django:main Oct 25, 2024
7 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants