tools: refactor js2c.py for maximal python3 compat by refack · Pull Request #25518 · nodejs/node

refack · 2019-01-15T16:54:37Z

Code before and after this PR generates the exact same output.
Tested with python2 (2.6 & 2.7) and python3 (3.6 & 3.7)
Passes make lint-py PYTHON=python2 and make lint-py PYTHON=python3
Whitespace changes are to comply with PEP8

/CC @nodejs/python

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

tools/js2c.py

cclauss · 2019-02-02T03:19:15Z

Fascinating what upstream did... v8/v8@7164251

BridgeAR · 2019-03-05T23:54:43Z

@nodejs/python @nodejs/tooling @nodejs/build PTAL

cclauss · 2019-03-06T03:38:01Z

Linter errors...

src/node_native_module.h:25:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/node_native_module.h:96:  Lines should be <= 80 characters long  [whitespace/line_length] [2]

refack · 2019-04-04T23:27:43Z

@nodejs/python PTAL I've addressed most of the comments, and cleaned this up some more.

cclauss · 2019-04-05T07:09:04Z

tools/js2c.py

+  # 2. turn pseudo-booleans strings into Booleans
+  config = re.sub('"true"', 'true', config)
+  config = re.sub('"false"', 'false', config)
+  return config


# four lines --> one... # normalize string literals from ' into " and turn pseudo-booleans strings into Booleans return config.replace('\'', '"').replace('"true"', 'true').replace('"false"', 'false')

cclauss

Great work!

refack · 2019-04-12T20:08:43Z

@nodejs/python @nodejs/build-files PTAL.

BridgeAR · 2019-04-15T16:50:04Z

This requires a rebase.

Trott · 2019-04-23T22:06:09Z

This looks good to me but my Python expertise is limited. Would much prefer it land with a review from @nodejs/python, but I'll give it a somewhat-rubber stamp LGTM if no one more informed steps in to review/approve/oppose/whatever it.

tools/js2c.py

* add explicit `--target` argument to enable succinct gyp declaration * simplify js2c semantics PR-URL: nodejs#25518 Reviewed-By: Christian Clauss <cclauss@me.com>

nodejs-github-bot · 2019-05-18T17:14:24Z

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

nodejs-github-bot · 2019-05-18T17:37:54Z

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

* add explicit `--target` argument to enable succinct gyp declaration * simplify js2c semantics PR-URL: #25518 Reviewed-By: Christian Clauss <cclauss@me.com>

joyeecheung · 2019-05-29T23:20:54Z

This landed as tools: refactor js2c.py for maximal python3 compat even though it contains changes that make every lib source uint16_t which is not required for python 3. compat, and it's not mentioned at all in the commit message..

refack · 2019-05-30T05:12:00Z

@joyeecheung you are right. That was not the intention of this PR and it's probably a rebasing artifact. I intended to move all the one-bye/two-byte changes to #27095 (we're we agreed to shelf that change)...
I'll post a correction ASAP.

refack · 2019-05-30T16:39:21Z

Regression fix in #27980

This comment has been minimized.

Sign in to view

nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jan 15, 2019

refack added the python PRs and issues that require attention from people who are familiar with Python. label Jan 15, 2019

thefourtheye reviewed Jan 16, 2019

View reviewed changes

cclauss mentioned this pull request Jan 29, 2019

build: ongoing list of actions for Python 3 compatibility #25789

Closed

63 tasks

cclauss reviewed Feb 2, 2019

View reviewed changes

tools/js2c.py Outdated Show resolved Hide resolved

refack force-pushed the tweak-js2c-2 branch from 395d098 to 9aad9f1 Compare February 15, 2019 00:23

refack mentioned this pull request Mar 19, 2019

doc: update supported platforms for Node.js 12 #26714

Closed

refack force-pushed the tweak-js2c-2 branch 4 times, most recently from 1e23681 to 14912d8 Compare April 4, 2019 23:03

refack mentioned this pull request Apr 4, 2019

src: simplify Javascript code embedding #27095

Closed

3 tasks

This comment has been minimized.

Sign in to view

cclauss reviewed Apr 5, 2019

View reviewed changes

cclauss approved these changes Apr 5, 2019

View reviewed changes

refack self-assigned this Apr 5, 2019

refack mentioned this pull request Apr 15, 2019

tools: js2c.py use ==/!= to compare str, bytes, and int literals #26979

Closed

4 tasks

refack force-pushed the tweak-js2c-2 branch 3 times, most recently from 2c3d1e7 to 2627fea Compare April 15, 2019 22:22

nodejs deleted a comment from nodejs-github-bot Apr 15, 2019

This comment has been minimized.

Sign in to view

nodejs deleted a comment from nodejs-github-bot Apr 15, 2019

joyeecheung reviewed Apr 23, 2019

View reviewed changes

tools/js2c.py Outdated Show resolved Hide resolved

refack force-pushed the tweak-js2c-2 branch from 2627fea to 6bd301c Compare May 3, 2019 22:46

This comment has been minimized.

Sign in to view

refack force-pushed the tweak-js2c-2 branch from 6bd301c to 1b2a9e7 Compare May 3, 2019 22:57

This comment has been minimized.

Sign in to view

tools: refactor js2c.py for maximal Python3 compatibility

bfbc035

* add explicit `--target` argument to enable succinct gyp declaration * simplify js2c semantics PR-URL: nodejs#25518 Reviewed-By: Christian Clauss <cclauss@me.com>

refack force-pushed the tweak-js2c-2 branch from 287b700 to bfbc035 Compare May 18, 2019 16:41

This comment has been minimized.

Sign in to view

refack merged commit bfbc035 into nodejs:master May 18, 2019

refack deleted the tweak-js2c-2 branch May 18, 2019 18:00

targos pushed a commit that referenced this pull request May 20, 2019

tools: refactor js2c.py for maximal Python3 compatibility

0db846f

* add explicit `--target` argument to enable succinct gyp declaration * simplify js2c semantics PR-URL: #25518 Reviewed-By: Christian Clauss <cclauss@me.com>

BridgeAR mentioned this pull request May 21, 2019

v12.3.0 proposal #27799

Merged

4 tasks

refack mentioned this pull request May 30, 2019

tools: fix js2c regression #27980

Closed

3 tasks

refack removed their assignment Jun 6, 2019

Uh oh!

Conversation

refack commented Jan 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cclauss commented Feb 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BridgeAR commented Mar 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cclauss commented Mar 6, 2019

Uh oh!

This comment has been minimized.

refack commented Apr 4, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

cclauss Apr 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cclauss left a comment

Choose a reason for hiding this comment

Uh oh!

refack commented Apr 12, 2019

Uh oh!

BridgeAR commented Apr 15, 2019

Uh oh!

This comment has been minimized.

Trott commented Apr 23, 2019

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented May 18, 2019

Uh oh!

nodejs-github-bot commented May 18, 2019

Uh oh!

joyeecheung commented May 29, 2019

Uh oh!

refack commented May 30, 2019

Uh oh!

refack commented May 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

refack commented Jan 15, 2019 •

edited

Loading

cclauss commented Feb 2, 2019 •

edited

Loading

BridgeAR commented Mar 5, 2019 •

edited

Loading

cclauss Apr 5, 2019 •

edited

Loading