Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

lib/path.js: Use '===' instead of '==' for comparison #7554

Closed
wants to merge 1 commit into from
Closed

lib/path.js: Use '===' instead of '==' for comparison #7554

wants to merge 1 commit into from

Conversation

stites
Copy link

@stites stites commented May 4, 2014

path: Use '===' instead of '==' for comparison

I've started rewriting node modules to practice typing and noticed that
you're evaluating this if (samePartsLength == 0) { condition without
type-checking. I thought I might submit a quick pull request to fix
this issue.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
I've started rewriting node modules to practice typing and noticed that you're evaluating this `if (samePartsLength == 0) {` condition without type-checking. I thought I might submit a quick pull request to fix this issue.
@Nodejs-Jenkins
Copy link

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit stites/node@eaa529944b68a4516ed9d43e3ff50b071ff759bc has the following error(s):

  • Commit message must indicate the subsystem this commit changes
  • Commit message line too long: 2

The following commiters were not found in the CLA:

  • Sam Stites

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

@stites stites changed the title Use '===' instead of '==' for comparison path.js: Use '===' instead of '==' for comparison May 4, 2014
@stites stites changed the title path.js: Use '===' instead of '==' for comparison lib/path.js: Use '===' instead of '==' for comparison May 4, 2014
@stites
Copy link
Author

stites commented May 4, 2014

Read the contributing doc - going to submit this as a new pull request on stable

@stites stites closed this May 4, 2014
@stites
Copy link
Author

stites commented May 4, 2014

Not sure which branch I should submit this to- the diff on 10.28 is pretty big. going to reopen this for guidance.

@stites stites reopened this May 4, 2014
@mks
Copy link

mks commented May 4, 2014

Type checking is not necessary, since samePartsLength is the minimum between two array lengths. I see a bigger problem with the implementation of the trim function, both in the windows and posix code path, that looks pretty broken.

@jasnell jasnell added the P-3 label Aug 13, 2015
@jasnell
Copy link
Member

jasnell commented Aug 13, 2015

Basic change looks good but PR is out of date and the change is a lower priority. Flagging it to get back to but marking as a low priority

@jasnell
Copy link
Member

jasnell commented Aug 15, 2015

Closing this here. New PR opened nodejs/node#2388

@jasnell jasnell closed this Aug 15, 2015
jasnell pushed a commit to jasnell/node that referenced this pull request Aug 15, 2015
silverwind pushed a commit to nodejs/node that referenced this pull request Aug 17, 2015
Per: nodejs/node-v0.x-archive#7554

Originally submitted by @stites

PR-URL: #2388
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Aug 19, 2015
Per: nodejs/node-v0.x-archive#7554

Originally submitted by @stites

PR-URL: nodejs#2388
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants