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

Pass in the "isGeneratedCode" flag from editorconfig options into syn… #40330

Conversation

mavasani
Copy link
Contributor

@mavasani mavasani commented Dec 12, 2019

…tax tree parse APIs in command line compiler and IDE syntax tree factory service and IGeneratedCodeRecognitionService.

Builds on top of #39051
Addresses #3705

…tax tree parse APIs in command line compiler and IDE syntax tree factory service.

Builds on top of dotnet#39051
Addresses dotnet#3705
@mavasani mavasani added this to the 16.5.P2 milestone Dec 12, 2019
@mavasani mavasani requested review from jasonmalinowski, agocke and a team December 12, 2019 14:40
@mavasani mavasani requested a review from a team as a code owner December 12, 2019 14:40
@mavasani
Copy link
Contributor Author

mavasani commented Dec 13, 2019

NOTE: Integration test failures should be resolved once #40363 is merged in. #40363 is merged in now.

Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Everything looks good except for the test: it seems like it's testing all the compiler or Workspace implementation APIs directly and thus not really testing the Workspace itself. For example, I wouldn't expect a test to be directly calling the syntax tree factories unless they're doing some specific memory testing around them. Seems like a better end-to-end test exists using the public APIs only like our other .editorconfig tests.

@mavasani
Copy link
Contributor Author

@dotnet/roslyn-compiler This is ready for review. Thanks!

Copy link
Member

@cston cston left a comment

Choose a reason for hiding this comment

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

Compiler changes LGTM.

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 7)

@JoeRobich
Copy link
Member

All CI passed. Overriding checks to merge.

@JoeRobich JoeRobich merged commit efb6a23 into dotnet:features/editorconfig-generated-code Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants