Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support for AdditionalFiles #1547

Merged

Conversation

kurobon-jp
Copy link
Contributor

Hello.
I propose this pull request as an answer to #1510.
github is unfamiliar, but I hope it helps a little.

@filipw
Copy link
Member

filipw commented Jul 9, 2019

thanks, could you add a test for this?

@kurobon-jp
Copy link
Contributor Author

Hello.
I added the AdditionalFiles test.

Copy link
Contributor

@savpek savpek left a comment

Choose a reason for hiding this comment

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

I think code side looks good, did you test in practise that these changes fixes #1547 ? Based on StyleCopAnalyzers repository documentation it indeed should but didn't test this by myself yet.

_workspace.RemoveAdditionalDocument(document.Id);
}

foreach (var file in additionalFiles) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: could you move { to the new line?

@kurobon-jp
Copy link
Contributor Author

I think code side looks good, did you test in practise that these changes fixes #1547 ? Based on StyleCopAnalyzers repository documentation it indeed should but didn't test this by myself yet.

Of course, I confirmed that the setting of stylecop.json was reflected.
But that may not be perfect.

@david-driscoll david-driscoll merged commit 4de169b into OmniSharp:master Jul 10, 2019
@david-driscoll
Copy link
Member

Thanks for the contribution @kurobon-jp!

@filipw
Copy link
Member

filipw commented Jul 10, 2019

👏

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants