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

lib/path: Add JSDoc Types #38186

Closed
wants to merge 1 commit into from
Closed

lib/path: Add JSDoc Types #38186

wants to merge 1 commit into from

Conversation

Skn0tt
Copy link
Contributor

@Skn0tt Skn0tt commented Apr 10, 2021

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. path Issues and PRs related to the path subsystem. labels Apr 10, 2021
@Skn0tt
Copy link
Contributor Author

Skn0tt commented Apr 10, 2021

I hope this PR and #38185 are of help. Let me know if I need to change anything :)

Ayase-252
Ayase-252 reviewed Apr 10, 2021
View reviewed changes
Copy link
Member

@Ayase-252 Ayase-252 left a comment

Choose a reason for hiding this comment

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

Thanks for contribution!

lib/path.js Outdated Show resolved Hide resolved
lib/path.js Outdated Show resolved Hide resolved
lib/path.js Outdated Show resolved Hide resolved
lib/path.js Outdated Show resolved Hide resolved
lib/path.js Outdated Show resolved Hide resolved
lib/path.js Outdated Show resolved Hide resolved
lib/path.js Outdated Show resolved Hide resolved
lib/path.js Outdated Show resolved Hide resolved
lib/path.js Outdated Show resolved Hide resolved
Ayase-252
Ayase-252 reviewed Apr 10, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
@Ayase-252
Copy link
Member

Hi, @Skn0tt

It seems there are still some errors from linters, would you mind fix them?

/home/runner/work/node/node/lib/path.js
Error:   1276:26  error  Trailing spaces not allowed                          no-trailing-spaces
Error:   1306:26  error  Trailing spaces not allowed                          no-trailing-spaces
Error:   1307:25  error  Trailing spaces not allowed                          no-trailing-spaces
Error:   1386:26  error  Trailing spaces not allowed                          no-trailing-spaces
Error:   1444:26  error  Trailing spaces not allowed                          no-trailing-spaces
Error:   1445:1   error  This line has a length of 86. Maximum allowed is 80  max-len

You can also run linters locally to detect linting problem by make lint or vcbuild lint based on your platform. Please refer to Running Tests section.

VoltrexKeyva
VoltrexKeyva reviewed Apr 10, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
VoltrexKeyva
VoltrexKeyva reviewed Apr 10, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
VoltrexKeyva
VoltrexKeyva reviewed Apr 10, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
VoltrexKeyva
VoltrexKeyva reviewed Apr 10, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
VoltrexKeyva
VoltrexKeyva reviewed Apr 10, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
VoltrexKeyva
VoltrexKeyva reviewed Apr 10, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
VoltrexKeyva
VoltrexKeyva reviewed Apr 10, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
VoltrexKeyva
VoltrexKeyva reviewed Apr 10, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
@targos targos added the typings label Apr 11, 2021
bmeck
bmeck approved these changes Apr 12, 2021
View reviewed changes
Copy link
Member

@bmeck bmeck left a comment

Choose a reason for hiding this comment

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

LGTM

targos
targos reviewed Apr 13, 2021
View reviewed changes
lib/path.js Outdated Show resolved Hide resolved
@targos targos self-assigned this Apr 14, 2021
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/37459/

targos pushed a commit that referenced this pull request Apr 14, 2021
@Skn0tt @targos
typings: add JSDoc types to lib/path
f1a21e5 
PR-URL: #38186
Reviewed-By: Bradley Farias <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos
Copy link
Member

targos commented Apr 14, 2021

Landed in f1a21e5. Thanks for the contribution!

@targos targos closed this Apr 14, 2021
BethGriggs pushed a commit that referenced this pull request Apr 15, 2021
@Skn0tt @BethGriggs
typings: add JSDoc types to lib/path
ba20adf 
PR-URL: #38186
Reviewed-By: Bradley Farias <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Apr 16, 2021
Merged
targos pushed a commit that referenced this pull request May 1, 2021
@Skn0tt @targos
typings: add JSDoc types to lib/path
e2c2f2b 
PR-URL: #38186
Reviewed-By: Bradley Farias <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Merged
danielleadams pushed a commit that referenced this pull request May 8, 2021
@Skn0tt @danielleadams
typings: add JSDoc types to lib/path
487fc31 
PR-URL: #38186
Reviewed-By: Bradley Farias <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos targos removed their assignment Oct 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ayase-252 Ayase-252 Ayase-252 left review comments

@VoltrexKeyva VoltrexKeyva VoltrexKeyva left review comments

@bmeck bmeck bmeck approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
needs-ci PRs that need a full CI run. path Issues and PRs related to the path subsystem. typings
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Skn0tt @Ayase-252 @nodejs-github-bot @targos @bmeck @VoltrexKeyva