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.

Sign up for GitHub

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

Jump to bottom

Remove jake (hopefully for real this time) #29085

Merged
merged 5 commits into from
Feb 20, 2019
Merged

Remove jake (hopefully for real this time) #29085

merged 5 commits into from
Feb 20, 2019

Conversation

weswigham
Copy link
Member

A couple of our build tasks will need to be updated, too; but I think this should be all that's needed in TS itself.

@weswigham
Copy link
Member Author

Oooooo that feeling when you put async functions in the gulpfile but node 6 doesn't support them natively. @rbuckton got a preference on how to handle that?

@sandersn
Copy link
Member

I would like to keep jake around until

> gulp
> gulp

Has the second invocation run fast, just like jake does today.

@weswigham
Copy link
Member Author

node 6 is in LTS until april, BTW. So April is roughly when we'll be OK ceasing to build on it.

@weswigham
Copy link
Member Author

weswigham commented Dec 19, 2018 •
edited
Loading

Well...
Now that's true. (re: multiple gulp invocations being slower)

@sandersn
Copy link
Member

This should wait on gulp (and gulp local if different) correctly building built/local/tsserver.js

@weswigham
Copy link
Member Author

@sandersn See #29447 😛

@rbuckton
Copy link
Member

Do you need to add the change to support using LKG on CI that we added to jake?

@weswigham
Copy link
Member Author

I'm doing something a little different - one second, just validating it a bit.

@weswigham
Fix gulpfile non-lkg build, add sanity-check build to posttest on CI,…
cab7e3c 
… accept older baseline style to go with lkgd build
@weswigham
Copy link
Member Author

There we go - rather than disabling lkg on CI (which leads to frustrating test diffs locally), I'm now running a sanity check built/local build on CI only after tests pass.

@sandersn
Copy link
Member

Node 6 is now out in favour of node 11, so that's one more obstacle out of the way.

@weswigham
Copy link
Member Author

Alright, looks like everything is now green. Anyone wanna peek over the CI logs to see that you're OK with the now gulp-based ones and give this the 👍?

@weswigham weswigham mentioned this pull request Feb 14, 2019
Merged
rbuckton
rbuckton approved these changes Feb 14, 2019
View reviewed changes
tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -8347,7 +8347,7 @@ declare namespace ts.server {
excludedFiles: ReadonlyArray<NormalizedPath>;
private typeAcquisition;
updateGraph(): boolean;
getExcludedFiles(): readonly NormalizedPath[];
getExcludedFiles(): ReadonlyArray<NormalizedPath>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this had already been addressed in #29783?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I un-addressed it to make the nightlies build again.

@weswigham
Copy link
Member Author

Alright. All build pipelines are on gulp-based scripts and the sync script for VS is setup to use gulp, too. Once this build passes now that I've synced with master and resolved the package.json conflict, I plan to merge this. That should unblock merging user suite PRs verbatim, and make the code coverage pipeline green again. This PR's been up for a week since approval, but if anyone has any final words before this merges, the next hour or so is what's left (barring catastrophic failures requiring a revert).

@weswigham weswigham merged commit b67f2d6 into microsoft:master Feb 20, 2019
@weswigham weswigham deleted the rm-jake branch February 20, 2019 23:32
@Kingwl
Copy link
Contributor

Kingwl commented Feb 21, 2019 •
edited
Loading

Wooo, it‘’s scared me.（bye jake）

@DanielRosenwasser DanielRosenwasser mentioned this pull request Mar 8, 2019
Closed
@alexeagle
Copy link
Contributor

@weswigham how do you have incremental builds now?

@weswigham
Copy link
Member Author

Our gulpfile is shelling out to tsc -b and not using gulp-typescript is how. 😐

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

@rbuckton rbuckton rbuckton approved these changes

@sandersn sandersn sandersn approved these changes

@ahejlsberg ahejlsberg Awaiting requested review from ahejlsberg

Assignees

@weswigham weswigham

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@weswigham @sandersn @rbuckton @Kingwl @alexeagle