FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: update incorrect links by noel046 · Pull Request #47085 · nodejs/node · GitHub

/ node Public

doc: update incorrect links - #47085

Closed
noel046 wants to merge 1 commit into
nodejs:mainfrom
noel046:Incorrect-links-#47070
ConversationCommits (1)ChecksFiles changed
Closed

doc: update incorrect links#47085
noel046 wants to merge 1 commit into
nodejs:mainfrom
noel046:Incorrect-links-#47070

Conversation

noel046 commented Mar 14, 2023 •
edited by aduh95
Loading

Copy link
Copy Markdown

Fixes: #47070

nodejs-github-bot commented Mar 14, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Mar 14, 2023
This was referenced Mar 21, 2023
Closed
Closed
Closed

deokjinkim commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

@noel046 Thank you for contribution. Commit title needs to be started with subsystem like doc: update incorrect links. Please follow Commit message guidelines.
https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#commit-message-guidelines

noel046 changed the title update incorrect links #47070 doc: update incorrect links Mar 24, 2023

deokjinkim commented Mar 25, 2023

Copy link
Copy Markdown
Contributor

@noel046 For now, PR title is only changed. But we need to change commit title(not PR of github) in git.

Trott commented Mar 26, 2023

Copy link
Copy Markdown
Member

@noel046 For now, PR title is only changed. But we need to change commit title(not PR of github) in git.

The merge commit will need to be removed too. I'll do that now and force push.

Trott force-pushed the Incorrect-links-#47070 branch from f9629a7 to b358df1 Compare March 26, 2023 02:59
mscdex mentioned this pull request Mar 31, 2023
Closed

Trott commented Apr 1, 2023

Copy link
Copy Markdown
Member

The CI failure is relevant. This breaks links in the single-page version of the doc.

Trott reviewed Apr 3, 2023
View reviewed changes
Comment thread doc/api/net.md
[`'drain'`]: #event-drain
[`'end'`]: #event-end
[`'error'`]: #event-error_1
[`'error'`]: #event-error-1

Trott Apr 3, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The current anchor exists in our HTML docs. The new one does not. What is the motivation for this change? Is it to fix something in the GitHub markdown rendering of this doc?

ovflowd Apr 3, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

+1, on our tooling we replace dots with nothing and underscores with hyphens. You can see it here: https://github.com/nodejs/node/blob/main/tools/doc/html.mjs#L426 (We do the same on nodejs.dev here https://github.com/nodejs/nodejs.dev/blob/main/util-node/createSlug.js#L1)

This was referenced Apr 7, 2023
Closed
Closed

mscdex commented Apr 7, 2023

Copy link
Copy Markdown
Contributor

Please also add a

Fixes: https://github.com/nodejs/node/issues/47070

to the commit message.

mscdex mentioned this pull request Apr 8, 2023
Closed
mscdex mentioned this pull request Apr 22, 2023
Closed

mscdex commented Apr 22, 2023

Copy link
Copy Markdown
Contributor

Can we get this merged soon? We've been getting more and more identical PRs for the same (or a subset of) changes.

ovflowd commented Apr 22, 2023

Copy link
Copy Markdown
Member

@mscdex did the issues Trott and me mentioned get resolved?

mscdex commented Apr 22, 2023

Copy link
Copy Markdown
Contributor

@ovflowd I don't know. However from my recollection we tend to give the first to submit a PR for a specific set of changes priority, but as this PR has been open for over a month with others willing to make the same changes, we need to decide whether to continue waiting or close this PR and give others a chance to make the changes needed.

aduh95 commented Sep 20, 2023

Copy link
Copy Markdown
Contributor

Superseded by #48194.

aduh95 closed this Sep 20, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Trott Trott left review comments
ovflowd ovflowd left review comments

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect links

7 participants

Back | FazBrowse Home | New Git URL