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

test: do not lint macros files (again) #24886

Closed
wants to merge 1 commit into from

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Dec 7, 2018

This PR is #24885 but without the modifications to deps/v8/src/js/macros.py

These three .py files do not contain any Python code. Instead they contain "macros" which are processed by js2c.py. Our Python linter (make lint-py) will properly flag these files as containing Python syntax errors. One solution would be to change the filename suffixes (.py -->.js2c) or we could add a linter directive (# noqa) to the comments of each file to tell our linter to ignore those files. This PR implements the latter.

@targos Your review please.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. tools Issues and PRs related to the tools directory. labels Dec 7, 2018
@Trott
Copy link
Member

Trott commented Dec 7, 2018

@Trott
Copy link
Member

Trott commented Dec 10, 2018

@cclauss
Copy link
Contributor Author

cclauss commented Dec 11, 2018

;-) These are COMMENTS in the code.

@targos
Copy link
Member

targos commented Dec 11, 2018

As this change only affects comments, lite CI is enough: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1893/ ✔️

@danbev
Copy link
Contributor

danbev commented Dec 11, 2018

Landed in 50cf6e7.

@danbev danbev closed this Dec 11, 2018
pull bot pushed a commit to zys-contrib/node that referenced this pull request Dec 11, 2018
PR-URL: nodejs#24886
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@cclauss cclauss deleted the Python-macros-again branch December 11, 2018 11:39
BethGriggs pushed a commit that referenced this pull request Dec 17, 2018
PR-URL: #24886
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@BethGriggs BethGriggs mentioned this pull request Dec 18, 2018
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
PR-URL: nodejs#24886
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
PR-URL: #24886
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
BethGriggs pushed a commit that referenced this pull request Feb 20, 2019
PR-URL: #24886
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
PR-URL: #24886
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants