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

Some of the test refactoring and readable baselining #56075

Merged
merged 7 commits into from
Oct 12, 2023
Merged

Conversation

sheetalkamat
Copy link
Member

Pulled out from #56074

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Oct 12, 2023
@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@@ -3450,7 +3450,6 @@ declare namespace ts {
getLanguageService(): never;
getHostForAutoImportProvider(): never;
getProjectReferences(): readonly ts.ProjectReference[] | undefined;
getTypeAcquisition(): TypeAcquisition;
Copy link
Member

Choose a reason for hiding this comment

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

Is this an intentional change?

Copy link
Member Author

Choose a reason for hiding this comment

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

It isnt test only change but i had done this as part of #56074 first and since it has no impact, left it there

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

Largely this looks good to me, though I just have the one comment about the API change, which doesn't feel intentional in a test-only change.

@@ -211,37 +214,37 @@ export function discoverTypings(
);
addInferredTypings(module, "Inferred typings from unresolved imports");
}
// Remove typings that the user has added to the exclude list
Copy link
Member Author

Choose a reason for hiding this comment

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

This is order change and shouldnt have any impact as such on functionality a359540

@@ -2640,10 +2640,6 @@ export class AutoImportProviderProject extends Project {
return PackageJsonAutoImportPreference.Off;
}

override getTypeAcquisition(): TypeAcquisition {
Copy link
Member Author

@sheetalkamat sheetalkamat Oct 12, 2023

Choose a reason for hiding this comment

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

This shouldnt be needed since default typeAcquisition is always disabled ae1b510

@sheetalkamat sheetalkamat merged commit feeb30d into main Oct 12, 2023
19 checks passed
@sheetalkamat sheetalkamat deleted the refactorTests branch October 12, 2023 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants