Skip to content
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

provide a .md template alongside the .rst one #94

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nanuxbe
Copy link

@nanuxbe nanuxbe commented Oct 7, 2024

As suggested in this forum thread and this mastodon thread and probably many other places (I know the topic has been brought up several times on the forum, only have this one link).

The .md file was mostly generated using pandoc (only alterating the head of the file).
Changes have also been made to the .rst template as well as DEP-1 in order to point to the markdown template, while the markdown template also points to the restructured text one

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.

Yes please! See also #85.

Here are minor tweaks I’d recommend, and a suggestion to also drop the forced line wrapping at 80 characters so the content is more readable.

final/0001-dep-process.rst Outdated Show resolved Hide resolved
(the same format as Django's documentation).
DEPs may be written in `reStructuredText <http://docutils.sourceforge.net/rst.html>`_
(the same format as Django's documentation) or
`MarkDown <https://www.markdownguide.org>` (the same as GitHub issues).

Each DEP should have the following parts:
Copy link
Member

Choose a reason for hiding this comment

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

I think it’d be worth updating the point about preambles below. Perhaps just removing the specifics:

A preamble -- metadata about the DEP […]

Copy link
Author

Choose a reason for hiding this comment

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

updated

conjunction with the content guidelines in [DEP
1](https://github.com/django/deps/final/0001-dep-process.rst), this
should make it easy for you to conform your own DEPs to the format
outlined below.
Copy link
Member

Choose a reason for hiding this comment

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

In #85 there’s also a mention of removing the wrapping to 80 characters, what do you think of updating this template accordingly?

template.md Outdated Show resolved Hide resolved
template.md Outdated Show resolved Hide resolved
TEMPLATE!**

To get the source this (or any) DEP, look at the top of the Github page
and click \"raw\".
Copy link
Member

Choose a reason for hiding this comment

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

Here and further through the rest of the doc, I think it’d be nice to remove the unnecessary escaping.

@nanuxbe
Copy link
Author

nanuxbe commented Oct 10, 2024

@thibaudcolas I'm personally in favour of everything mentioned in #85 and the original list but since none of those seemed to have reached a consensus, my goal for this PR was to be as minimal as possible in order to be able to merge it as quick as possible.

If we can get a strong support behind all those changes, I'd be more than happy to have it here.

Then I would also suggest removing the manual TOC in the .md file since, GitHub has this out of the box as a dropdown.
image
image

Co-authored-by: Thibaud Colas <[email protected]>
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.

Makes sense to me! Looking good

@nanuxbe
Copy link
Author

nanuxbe commented Oct 19, 2024

@thibaudcolas

Makes sense to me!

Which part? the part about not doing more in this particular PR or the part about doing more?

@thibaudcolas
Copy link
Member

the part about keeping things simple until there’s more support :)

Copy link

@gvangool gvangool left a comment

Choose a reason for hiding this comment

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

About the Markdown vs MarkDown, I updated it to be consistent (I think the lowercase d is the correct casing).

Also suggested some syntax fixes for the rST links.

final/0001-dep-process.rst Outdated Show resolved Hide resolved
final/0001-dep-process.rst Outdated Show resolved Hide resolved
final/0001-dep-process.rst Outdated Show resolved Hide resolved
template.md Outdated Show resolved Hide resolved
template.rst Outdated Show resolved Hide resolved
template.md Outdated Show resolved Hide resolved
Copy link
Member

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

Yep, no reason not to allow folks to use Markdown.

@@ -295,21 +295,25 @@ DEP format

To save everyone time reading DEPs, they need to follow a common format
and outline; this section describes that format. In most cases, it's probably
easiest to start with copying the provided `DEP template <../template.rst>`_,
easiest to start with copying one of the provided DEP templates
(`rst template <../template.rst>` - `md template <../template.md>`)_,

Choose a reason for hiding this comment

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

This doesn't render correctly. Looking at the old link, I think it should be:

Suggested change
(`rst template <../template.rst>` - `md template <../template.md>`)_,
(`rst template <../template.rst>`_ - `md template <../template.md>`_),

Comment on lines +315 to +316
or `Markdown frontmatter <https://github.com/Kernix13/markdown-cheatsheet/blob/master/frontmatter.md>`_
depending on which template you are using.

Choose a reason for hiding this comment

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

I think it's clear enough that which you use would depend on the template chosen:

Suggested change
or `Markdown frontmatter <https://github.com/Kernix13/markdown-cheatsheet/blob/master/frontmatter.md>`_
depending on which template you are using.
or `Markdown frontmatter <https://github.com/Kernix13/markdown-cheatsheet/blob/master/frontmatter.md>`_.

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.

5 participants