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.

Sign up for GitHub

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

Jump to bottom

fs: fix link rot in fs.md #8940

Closed
wants to merge 3 commits into from
Closed

fs: fix link rot in fs.md #8940

wants to merge 3 commits into from

Conversation

suramya-shah
Copy link

@suramya-shah suramya-shah commented Oct 5, 2016 •
edited by Fishrock123
Loading

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Updated line 2197
fixes #8825

Update fs.md
d3569b3 
 Updated line 2197
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Oct 5, 2016
suramya-shah
suramya-shah commented Oct 5, 2016
View reviewed changes
doc/api/fs.md
@@ -2194,7 +2194,7 @@ The following constants are meant for use with the [`fs.Stats`][] object's
[MDN-Date]: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Date
[Readable Stream]: stream.html#stream_class_stream_readable
[Writable Stream]: stream.html#stream_class_stream_writable
[inode]: http://www.linux.org/threads/intro-to-inodes.4130
[inode]:https://en.wikipedia.org/wiki/Inode
Copy link
Author

Choose a reason for hiding this comment

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

changed from http://www.linux.org/threads/intro-to-inodes.4130 to
https://en.wikipedia.org/wiki/Inode

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need a space after the :?

Copy link
Author

Choose a reason for hiding this comment

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

Yes I merged a pull request for the same

@Fishrock123
Copy link
Contributor

Also could you change the commit message to be more like: fs: fix link rot in fs.md?

@suramya-shah suramya-shah changed the title Update fs.md fs: fix link rot in fs.md Oct 6, 2016
addaleax
addaleax approved these changes Oct 6, 2016
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Looks good! Squashing the commits + fixing up the commit message can be done when merging the PR.

lpinca
lpinca approved these changes Oct 6, 2016
View reviewed changes
Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM

@yorkie
Copy link
Contributor

yorkie commented Oct 6, 2016

By the way, @ss22ever could you double check on others which we can use wikipedia instead of?

jasnell
jasnell approved these changes Oct 6, 2016
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM

jasnell pushed a commit that referenced this pull request Oct 6, 2016
@jasnell
doc: fixup link in fs.md
3945943 
PR-URL: #8940
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@jasnell
Copy link
Member

jasnell commented Oct 6, 2016

Landed in 3945943. Thank you!

@jasnell jasnell closed this Oct 6, 2016
jasnell pushed a commit that referenced this pull request Oct 6, 2016
@jasnell
doc: fixup link in fs.md
09987d2 
PR-URL: #8940
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@imyller imyller mentioned this pull request Oct 7, 2016
Closed
@suramya-shah
Copy link
Author

@Yorki as told by you I double checked the links the rest of them are working fine here.

@lpinca
Copy link
Member

lpinca commented Oct 7, 2016

@ss22ever I'm not sure but I think @yorkie wanted to check if there were better resources than Wikipedia.

@suramya-shah
Copy link
Author

suramya-shah commented Oct 7, 2016 •
edited
Loading

@lpinca oh pardon then if thats so should I make a PR for the ones that I have found?

@addaleax
Copy link
Member

addaleax commented Oct 7, 2016

fwiw, I read @yorkie’s comment the opposite way… i.e. checking which other links could be replaced by links to Wikipedia. 😄

@suramya-shah
Copy link
Author

Yeah even I and then I checked all the links to ensure that :D @addaleax

@lpinca
Copy link
Member

lpinca commented Oct 7, 2016

2 vs 1 I'm probably the one that misread / did not understand 😄. @ss22ever I think that it is fine as is, maybe @yorkie can clarify later.

@suramya-shah
Copy link
Author

Is this PR merged ? If yes why can I not see the merged icon ?

@addaleax
Copy link
Member

addaleax commented Oct 7, 2016

@ss22ever Yes! “Landed in 3945943” means that it’s been merged and that the commit in which it has been merged into commit 3945943 on master.

The “Merged” icon is missing for technical reasons related to our process of merging PRs.

Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
@Fishrock123
doc: fixup link in fs.md
c31a317 
PR-URL: #8940
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fishrock123 Fishrock123 Fishrock123 left review comments

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@lpinca lpinca lpinca approved these changes

@yorkie yorkie yorkie approved these changes

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

doc: dead external link in fs.md
8 participants
@suramya-shah @Fishrock123 @yorkie @jasnell @lpinca @addaleax @MylesBorins @nodejs-github-bot