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

feat(gyp): update gyp to v0.9.6 #2481

Merged
merged 1 commit into from Aug 23, 2021
Merged

Conversation

gengjiawen
Copy link
Member

Checklist
  • npm install && npm test passes
  • tests are included
  • documentation is changed or added
  • commit message follows commit guidelines
Description of change

@rvagg
Copy link
Member

rvagg commented Aug 10, 2021

couple of windows issues there but rubber stamp from me if you can figure those out
I suppose I should cut a release after this! we have a fair bit built up

gyp/pylib/gyp/easy_xml.py Outdated Show resolved Hide resolved
@rvagg rvagg mentioned this pull request Aug 12, 2021
@gengjiawen gengjiawen changed the title feat(gyp): update gyp to v0.9.4 feat(gyp): update gyp to v0.9.5 Aug 18, 2021
@@ -67,7 +74,8 @@ def EncodeRspFileList(args):
program = call + " " + os.path.normpath(program)
else:
program = os.path.normpath(args[0])
return program + " " + " ".join(QuoteForRspFile(arg) for arg in args[1:])
return (program + " " +
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks different lint rules result in this. Should we skip gyp lib in this repo or unify the rule ? @cclauss

Run flake8 . --ignore=E203,W503  --max-complexity=101 --max-line-length=88 --show-source --statistics
./gyp/pylib/gyp/msvs_emulation.py:77:27: W504 line break after binary operator
    return (program + " " +
                          ^
1     W504 line break after binary operator

Copy link
Contributor

Choose a reason for hiding this comment

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

Put the + at the beginning of the next line.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cclauss

Should we add this rule in https://github.com/nodejs/gyp-next ?

Also should we disable flake8 test for this repo since gyp sources is an upstream project ?

@gengjiawen gengjiawen changed the title feat(gyp): update gyp to v0.9.5 feat(gyp): update gyp to v0.9.6 Aug 23, 2021
@gengjiawen
Copy link
Member Author

@rvagg This id ready for review.

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

rubber stamp from me
changes to path handling on windows always makes me nervous since we get so many reports related to paths but 🤞 I trust you folks know best!

Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

LGTM

@gengjiawen gengjiawen merged commit ed9a9ed into nodejs:master Aug 23, 2021
@gengjiawen gengjiawen deleted the bump_gyp branch August 23, 2021 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants