Skip to content

tools: do not lint tools/inspector_protocol,tools/markupsafe - #24882

Closed
cclauss wants to merge 2 commits into
nodejs:masterfrom
cclauss:Py3-for-tools-inspector_protocol
Closed

cclauss wants to merge 2 commits into
nodejs:masterfrom
cclauss:Py3-for-tools-inspector_protocol

Conversation

@cclauss

@cclauss cclauss commented Dec 7, 2018

Copy link
Copy Markdown
Contributor
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 the tools Issues and PRs related to the tools directory. label Dec 7, 2018
@Trott

ghost commented Dec 7, 2018 •

Copy link
Copy Markdown
Member

@nodejs/v8-inspector Should these changes land in our repo or should they be upstreamed to https://chromium.googlesource.com/deps/inspector_protocol/? (If the latter, should this live more properly in deps ultimately?)

@cclauss
cclauss force-pushed the Py3-for-tools-inspector_protocol branch from daf9dc6 to aeb4481 Compare December 7, 2018 07:40
@cclauss cclauss changed the title tools: prepare tools/inspector_protocol for Python 3 tools: do not lint tools/inspector_protocol Dec 7, 2018
@cclauss

ghost commented Dec 7, 2018 •

Copy link
Copy Markdown
Contributor Author

@bricss Instead of a 👍 could I please ask you to Review changes because that will give project maintainers more confidence that you has read through the actual changes and believe that those changes improve the state of the project?

Anyone can review a GitHub pull request:

  1. Scroll up on this page and click on Files changed just above the initial commit message.
  2. Click on the green button Review changes
  3. Click on Comment or Approve or Request changes

Thanks for taking the time to go through these step which should help maintainers.

$ __make lint-py__
```
./tools/markupsafe/_compat.py:21:17: F821 undefined name 'unicode'
    text_type = unicode
                ^
./tools/markupsafe/_compat.py:22:26: F821 undefined name 'unicode'
    string_types = (str, unicode)
                         ^
./tools/markupsafe/_compat.py:24:23: F821 undefined name 'long'
    int_types = (int, long)
                      ^
```
@cclauss cclauss changed the title tools: do not lint tools/inspector_protocol tools: do not lint tools/inspector_protocol,tools/markupsafe Dec 7, 2018
@Trott

ghost commented Dec 7, 2018

Copy link
Copy Markdown
Member

@Trott

ghost commented Dec 10, 2018 •

Copy link
Copy Markdown
Member

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19370/ ✔️

@Trott

ghost commented Dec 10, 2018

Copy link
Copy Markdown
Member

Landed in 5652cb0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants