Skip to content

Conversation

nolanmar511
Copy link
Contributor

@nolanmar511 nolanmar511 commented Apr 25, 2019

Specifying target in the main tsconfig.json was not necessary to make system tests pass, but seemed prudent given the problem we had with the benchmark.

@nolanmar511 nolanmar511 changed the title chore: fix Node 6 system test by 6 adding a target to benchmark's tsc… chore: fix Node 6 system test by 6 adding a target to benchmark's tsconfig Apr 25, 2019
@kalyanac kalyanac changed the title chore: fix Node 6 system test by 6 adding a target to benchmark's tsconfig chore: fix Node 6 system test by adding a target to benchmark's tsconfig Apr 25, 2019
@nolanmar511 nolanmar511 changed the title chore: fix Node 6 system test by adding a target to benchmark's tsconfig chore: fix Node 6 system test by specifying target in tsconfig Apr 25, 2019
@nolanmar511 nolanmar511 changed the title chore: fix Node 6 system test by specifying target in tsconfig fix: fix Node 6 system test by specifying target in tsconfig Apr 25, 2019
@nolanmar511 nolanmar511 changed the title fix: fix Node 6 system test by specifying target in tsconfig fix: Specify target in tsconfig to fix Node 6 system test Apr 25, 2019
@nolanmar511 nolanmar511 requested a review from kalyanac April 25, 2019 23:16
@kalyanac
Copy link

@nolanmar511 Does this force using the target flag value es2015 for all node versions?

@ofrobots
Copy link
Contributor

Is there a strong reason to continue supporting Node 6? It will reach EOL on Tuesday.

@nolanmar511
Copy link
Contributor Author

For right now, I think merging just to stop alerts from our continuous tests might make sense. But removing the system tests for Node 6 instead.

But yes, this change would cause all versions of Node.js to use the javascript transpiled with the target es2015 -- the npm package only includes 1 version of javascript files and those are used by all versions of Node.js.

@kalyanac
Copy link

ignoring the npmaudit fail for now.

@kalyanac kalyanac merged commit 413a371 into google:master Apr 26, 2019
@ofrobots
Copy link
Contributor

To make sure, you're planning to remove the es2015 target config at the time you drop support for Node 6?

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.

3 participants