Skip to content

Don't npm install the parent project with no args #35359

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

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

jablko
Copy link
Contributor

@jablko jablko commented Nov 26, 2019

No description provided.

@jablko jablko force-pushed the patch-30 branch 2 times, most recently from b4e13b6 to f2540d3 Compare November 29, 2019 22:25
@jablko jablko force-pushed the patch-30 branch 3 times, most recently from fd9652c to 34c331e Compare January 25, 2020 20:23
@sandersn sandersn added the Housekeeping Housekeeping PRs label Feb 1, 2020
@jablko jablko force-pushed the patch-30 branch 4 times, most recently from 03dbecc to 0959df8 Compare February 9, 2020 17:19
@jablko jablko force-pushed the patch-30 branch 2 times, most recently from bb96781 to 78ade52 Compare February 11, 2020 18:59
@@ -78,7 +78,7 @@ namespace Harness {
exec("npm", ["i", "--ignore-scripts"], { cwd, timeout: timeout / 2 }); // NPM shouldn't take the entire timeout - if it takes a long time, it should be terminated and we should log the failure
}
const args = [path.join(IO.getWorkspaceRoot(), "built/local/tsc.js")];
if (types) {
if (types?.length) {
args.push("--types", types.join(","));
Copy link
Member

Choose a reason for hiding this comment

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

this is not correct; args needs to include --types even if the list is empty. npm install is OK to skip though.

I think this will make the diffs in the baselines go away, unfortunately. The real fix for those projects is to add types entries to their tsconfigs.

@@ -1,4 +1,4 @@
{
"cloneUrl": "https://github.com/Microsoft/TypeScript-React-Starter",
"cloneUrl": "https://github.com/Microsoft/TypeScript-WeChat-Starter.git",
Copy link
Member

Choose a reason for hiding this comment

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

ha ha oh no

@sandersn sandersn merged commit bf37065 into microsoft:master Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping Housekeeping PRs
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants