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

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

Closed

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Dec 7, 2018

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
Copy link
Member

Trott commented Dec 7, 2018

@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
Copy link
Contributor Author

cclauss commented Dec 7, 2018

@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
Copy link
Member

Trott commented Dec 7, 2018

@Trott
Copy link
Member

Trott commented Dec 10, 2018

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

@Trott
Copy link
Member

Trott commented Dec 10, 2018

Landed in 5652cb0

@Trott Trott closed this Dec 10, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Dec 10, 2018
PR-URL: nodejs#24882
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@cclauss cclauss deleted the Py3-for-tools-inspector_protocol branch December 10, 2018 04:59
BethGriggs pushed a commit that referenced this pull request Dec 17, 2018
PR-URL: #24882
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@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#24882
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
PR-URL: #24882
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
BethGriggs pushed a commit that referenced this pull request Feb 20, 2019
PR-URL: #24882
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
PR-URL: #24882
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
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