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

doc: specify that longDescription should be Markdown #188805

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

ncfavier
Copy link
Member

See NixOS/nixos-search#525 for context.

In the spirit of RFC 72, document that longDescription is in Markdown format (should this be CommonMark?).

This is purely a documentation change because I don't think there are currently any consumers of longDescription that rely on it being a particular format besides plain text.

Producers, on the other hand, seem to have been using the Markdown convention, if any.

@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label Aug 29, 2022
@ncfavier ncfavier requested review from roberth, pennae and jtojnar August 29, 2022 12:23
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Aug 29, 2022
@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Aug 29, 2022

Please reference RFC72 in the commit message so we have a paper trail for the reasoning behind the change.
(I recommend writing the motivation in the commit message and re-using it for the pull-request description.)

@ncfavier ncfavier force-pushed the longDescription-format branch from 5162793 to f2b012d Compare August 29, 2022 12:55
Copy link
Contributor

@pennae pennae left a comment

Choose a reason for hiding this comment

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

maybe we should link to the nixpkgs chapter of markdown extensions instead, but using plain commonmark for long descriptions also also perfectly fine.

@bobby285271 bobby285271 added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Aug 29, 2022
@@ -80,7 +80,7 @@ Right: `"A library for decoding PNG images"`

### `longDescription` {#var-meta-longDescription}

An arbitrarily long description of the package.
An arbitrarily long description of the package in [CommonMark](https://commonmark.org) Markdown format.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
An arbitrarily long description of the package in [CommonMark](https://commonmark.org) Markdown format.
An arbitrarily long description of the package in [CommonMark](https://commonmark.org) Markdown.

Either use this suggestion, or add "the".

Copy link
Member

Choose a reason for hiding this comment

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

Or to follow pennae's suggestion: Nixpkgs Markdown.

In the spirit of RFC 72, document that longDescription is in CommonMark.
@ncfavier ncfavier force-pushed the longDescription-format branch from f2b012d to 2f88279 Compare August 29, 2022 18:19
@ncfavier
Copy link
Member Author

I don't think we want to use any extensions in package descriptions, so CommonMark should be fine.

@SuperSandro2000
Copy link
Member

This is purely a documentation change because I don't think there are currently any consumers of longDescription that rely on it being a particular format besides plain text.

Yes, currently it is plain text.

@@ -80,7 +80,7 @@ Right: `"A library for decoding PNG images"`

### `longDescription` {#var-meta-longDescription}

An arbitrarily long description of the package.
An arbitrarily long description of the package in [CommonMark](https://commonmark.org) Markdown.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
An arbitrarily long description of the package in [CommonMark](https://commonmark.org) Markdown.
An arbitrarily long description of the package in [CommonMark Markdown](https://commonmark.org).

Copy link
Member

Choose a reason for hiding this comment

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

That makes it look too much like there is a thing called CommonMark Markdown, maybe use something like the following instead:

Suggested change
An arbitrarily long description of the package in [CommonMark](https://commonmark.org) Markdown.
An arbitrarily long description of the package in [CommonMark](https://commonmark.org) Markdown dialect.

Copy link
Member

Choose a reason for hiding this comment

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

@jtojnar in that case, add "the".

Suggested change
An arbitrarily long description of the package in [CommonMark](https://commonmark.org) Markdown.
An arbitrarily long description of the package in the [CommonMark](https://commonmark.org) Markdown dialect.

Copy link
Member

Choose a reason for hiding this comment

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

My latest suggestion was 50% ironic and this is 100% my last suggestion.

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 the sentence is fine as it is and the bikeshed will stay purple.

Copy link
Member

Choose a reason for hiding this comment

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

No, no, no, the bikeshed must be blue. Anything else is ridiculous.

@roberth roberth merged commit 507d35c into NixOS:master Aug 29, 2022
@ncfavier ncfavier deleted the longDescription-format branch August 29, 2022 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 12.approvals: 2 This PR was reviewed and approved by two reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants