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

Update CONTRIBUTING.md #33523

Merged
merged 6 commits into from May 10, 2024
Merged

Update CONTRIBUTING.md #33523

merged 6 commits into from May 10, 2024

Conversation

estelle
Copy link
Member

@estelle estelle commented May 9, 2024

If someone doesn't read the intro, and just scans the bullets, the javascript file seemed odd. Added a bit of text in case someone does that (people tend to read bullets, not prose, so it's likely.

Please add a link on link 85 - the url is missing in the href.

@estelle estelle requested a review from a team as a code owner May 9, 2024 20:08
@estelle estelle requested review from bsmth and removed request for a team May 9, 2024 20:08
@github-actions github-actions bot added the size/xs 0-5 LoC changed label May 9, 2024
4. Click the edit (pencil) button

From there, the GitHub UI will walk you through the rest by creating a [fork](https://docs.github.com/en/get-started/quickstart/fork-a-repo) and a branch to commit your changes to.
After you have made changes to your branch, the goal is to open a [pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests) for your changes to be incorporated.
After you have made changes to your branch, open a [pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests) with your changes to be incorporated.

A pull request represents the work you want to be reviewed, approved, and merged into the `main` branch of the MDN repository.
See the [Creating a pull request](#creating-a-pull-request) for more details on creating and handling pull requests successfully.
Copy link
Member Author

Choose a reason for hiding this comment

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

on the next line, the href if empty

Copy link
Collaborator

Choose a reason for hiding this comment

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

@estelle I fixed and then un-fixed this. This is a reference link - so the definitions are done at the bottom of the page.

@dipikabh / @bsmth While this reference works, it is deprecated by tools such as prettier, because as an editor it is hard to tell if the link is omitted or satisfied by a later reference. Generally they look great, but they make the doc hard to read.

Prettier formats for this case to [get in touch with us][get in touch with us], and I tend to use shorter references, so I'd have something like [get in touch with us][get_in_touch].

I'm not advocating this - I think reference links should only be used if you have like 3 instances of a link in a page. Otherwise use normal links. And if you have to use a reference link, use the prettier format

Copy link
Collaborator

Choose a reason for hiding this comment

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

PS note some links are broken - specifically those on line 13 as the use this format: - [Writing guidelines][]. They'd work if you instead had - [Writing guidelines]. Not fixing, as I want some discussion of ^^^

Copy link
Member

Choose a reason for hiding this comment

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

The reason for using the reference style links here is for Markdownlint to catch links in this format in the rest of content:

see [this page](https://developer.mozilla.org/en-US/docs/Web/JavaScript)

And correct it to:

see [this page](/en-US/docs/Web/JavaScript)

This is fine everywhere else, but breaks in the .md files intended to be read on GitHub. As a workaround, we use this format:

See the [JavaScript landing page][] for more info...

[javascript landing page]: https://developer.mozilla.org/en-US/docs/Web/JavaScript

This was introduced here:

https://github.com/mdn/content/pull/21432/files


links are broken - specifically those on line 13 as the use this format [Writing guidelines][]

It looks weird, but it works okay: https://github.com/mdn/content/blob/main/CONTRIBUTING.md#getting-started

Here's the spec for this "shortcut reference link" syntax: https://github.github.com/gfm/#shortcut-reference-link (ex 575)

@github-actions github-actions bot added size/s 6-50 LoC changed and removed size/xs 0-5 LoC changed labels May 9, 2024
CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Approving, but see https://github.com/mdn/content/pull/33523/files#r1596079745 - "reference links suck and there are still some reference link bugs".

CONTRIBUTING.md Outdated Show resolved Hide resolved
@estelle
Copy link
Member Author

estelle commented May 10, 2024

I reverted my [] removal, but kept the grammar changes that were approved.

@estelle estelle merged commit 72073e4 into main May 10, 2024
8 checks passed
@estelle estelle deleted the estelle-patch-6 branch May 10, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/s 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants