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

fs: Replace checkPosition with validateInteger #33277

Closed
wants to merge 2 commits into from

Conversation

rickyes
Copy link
Contributor

@rickyes rickyes commented May 7, 2020

/cc @ZYSzys

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the fs Issues and PRs related to the fs subsystem / file system. label May 7, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@rickyes
Copy link
Contributor Author

rickyes commented May 16, 2020

It's ready.

@ZYSzys ZYSzys added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 17, 2020
@addaleax
Copy link
Member

@rickyes Could you rebase this against master?

@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 20, 2020
@rickyes
Copy link
Contributor Author

rickyes commented May 20, 2020

@rickyes Could you rebase this against master?

done

@rickyes
Copy link
Contributor Author

rickyes commented May 21, 2020

ping @addaleax . A check being canceled does not seem to be related to this PR.

@addaleax
Copy link
Member

@rickyes This still needs to be rebased using git rebasegit merge doesn’t work for our CI.

Let me know if you’d prefer that somebody else does that, that’s no problem.

@rickyes
Copy link
Contributor Author

rickyes commented May 21, 2020

@rickyes This still needs to be rebased using git rebasegit merge doesn’t work for our CI.

Let me know if you’d prefer that somebody else does that, that’s no problem.

done

@nodejs-github-bot
Copy link
Collaborator

@rickyes
Copy link
Contributor Author

rickyes commented May 21, 2020

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

Looks like jenkinsci has a problem.
https://ci.nodejs.org/job/node-test-commit-osx/nodes=osx1015/34277/console

@nodejs-github-bot
Copy link
Collaborator

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

@rickyes
Copy link
Contributor Author

rickyes commented May 21, 2020

ZYSzys pushed a commit that referenced this pull request May 22, 2020
PR-URL: #33277
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
@ZYSzys
Copy link
Member

ZYSzys commented May 22, 2020

Landed in 49f549e.

@ZYSzys ZYSzys closed this May 22, 2020
@rickyes rickyes deleted the refactor-checkPosition branch May 22, 2020 01:53
codebytere pushed a commit that referenced this pull request Jun 18, 2020
PR-URL: #33277
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
@codebytere codebytere mentioned this pull request Jun 28, 2020
codebytere pushed a commit that referenced this pull request Jul 8, 2020
PR-URL: #33277
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
@codebytere codebytere mentioned this pull request Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants