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: fix incorrect links #47089

Closed
wants to merge 2 commits into from
Closed

Conversation

Robot-dev11
Copy link

Changes in BUILDING.md, net.md, process.md and worker_threads.md file for issue #47070

Fixes: #47070

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot 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
@Robot-dev11 Robot-dev11 reopened this Mar 14, 2023
@Robot-dev11 Robot-dev11 reopened this Mar 14, 2023
@MoLow
Copy link
Member

MoLow commented Mar 14, 2023

@Robot-dev11 thanks for your contribution! the commit message should describe the fix directly, and the subsystem is doc.
I suggest something like doc: fix incorrect links

@MoLow MoLow mentioned this pull request Mar 14, 2023
@Robot-dev11 Robot-dev11 changed the title subsystem: changes for good-first-issue incorrect links #47070 doc: fix incorrect links Mar 14, 2023
@Robot-dev11
Copy link
Author

Robot-dev11 commented Mar 14, 2023

Hi @MoLow,

I have changed the commit message. Can you review please

Thanks,
Giriraj Soni

@MoLow
Copy link
Member

MoLow commented Mar 14, 2023

@Robot-dev11 the change you have performed only changed the PR name.
you should amend your commit and force push it

@Robot-dev11
Copy link
Author

Yeah, @MoLow Sorry for that i have also done amend for commit and force to push it

@Robot-dev11 Robot-dev11 force-pushed the issue-47070 branch 2 times, most recently from 18481f3 to 51ea3ff Compare March 15, 2023 03:38
@Robot-dev11
Copy link
Author

Hi,
Can anyone review it because some test cases failed and i have changed the commit message to avoid failure in test cases.

Thanks

Copy link
Member

Choose a reason for hiding this comment

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

It seems the changes in this file are not required, these changes are making the workflow fail, I tested locally. you can test it locally too by running make doc and check any failures

@MoLow MoLow mentioned this pull request Mar 15, 2023
Giriraj Soni added 2 commits March 15, 2023 19:27
Changes in BUILDING.md, net.md, process.md and worker_threads.md file for issue nodejs#47070

Fixes: nodejs#47070
Revert changes of process.md and worker_threads.md
Fixes: nodejs#47070
@mscdex
Copy link
Contributor

mscdex commented Mar 21, 2023

There is already a PR open for these changes here.

[`'error'`]: #event-error_1
[`'error'`]: #event-error-1
Copy link
Member

Choose a reason for hiding this comment

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

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

@Robot-dev11 Robot-dev11 closed this Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect links
6 participants