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

Close #10942 #11163

Closed
wants to merge 1,517 commits into from
Closed

Close #10942 #11163

wants to merge 1,517 commits into from

Conversation

Hosch250
Copy link
Contributor

@Hosch250 Hosch250 commented May 7, 2016

Diagnostic/quick fix for adding braces to if's, else's, for's, foreach's, while's, do-while's, and using's.

Close #10942

pawchen and others added 30 commits April 19, 2016 17:55
Update FX dependencies to build 24011
Remove System.AppContext to Roslyn.VisualStudio.InteractiveComponents
invoke next handler in case if no signature help providers can be fou…
Fix compiler crash in broken code in pattern matching
Update TargetFrameworkVersions.
Add DSR.Portable project.json to .csproj
{
private static readonly LocalizableString s_localizableTitle =
new LocalizableResourceString(nameof(FeaturesResources.AddBraces), FeaturesResources.ResourceManager,
typeof (FeaturesResources));
Copy link
Member

Choose a reason for hiding this comment

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

No space around typeof.

@CyrusNajmabadi
Copy link
Member

A few more tweaks to make. almost there!

var diagnosticSpan = diagnostic.Location.SourceSpan;
var statement = root.FindNode(diagnosticSpan);

SyntaxNode newBlock = null;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: my preferred patternfor this is to have a helper method which returns the new block. That way in the switch all the cases can just return instead of needing to assign then break. Your call on if you want that .

@CyrusNajmabadi
Copy link
Member

👍 with the change to make this hidden by default.

If you're still having test problems i'm happy to take a look.

@Hosch250
Copy link
Contributor Author

Hosch250 commented May 8, 2016

I haven't tried it on my machine lately, but the tests are failing on this build as well, indicating the problem is still there.

@CyrusNajmabadi
Copy link
Member

Ok! I'll look at the tests!

@CyrusNajmabadi
Copy link
Member

i burnt a finger cooking though. So i may have to wait till later. it's not fun typing right now :)

@CyrusNajmabadi
Copy link
Member

Also, i just talked with @Pilchie . We'd like this to be agianst Future. I'm happy to rebase things for you if you're not famliar with doing that.

@Hosch250
Copy link
Contributor Author

Hosch250 commented May 8, 2016

I'm not familiar doing that, so that would be nice. Wait for my next commit, please, after I respond to your comments.

@CyrusNajmabadi
Copy link
Member

Sure! can you give me permissions to push to your repo?

@Hosch250
Copy link
Contributor Author

Hosch250 commented May 8, 2016

Sure. Let me look at my settings.

@CyrusNajmabadi
Copy link
Member

It should be under settings -> collaborators

@Hosch250
Copy link
Contributor Author

Hosch250 commented May 8, 2016

Found it. You have push access.

@Hosch250
Copy link
Contributor Author

Hosch250 commented May 8, 2016

Just manually testing it before I commit.

@CyrusNajmabadi
Copy link
Member

Ok. Let me know when i can make changes. Because i'll be rewriting history, i'll need to do a force push. And i don't want to do that while you're in the middle of things.

@Hosch250
Copy link
Contributor Author

Hosch250 commented May 8, 2016

Go ahead.

@Hosch250
Copy link
Contributor Author

Hosch250 commented May 8, 2016

Once you do this, I will need to close this and re-PR against Future?

@Hosch250
Copy link
Contributor Author

Hosch250 commented May 8, 2016

@CyrusNajmabadi I looked in the log files, and this is the error:

xUnit.net Console Runner (32-bit .NET 4.0.30319.42000)
System.InvalidOperationException: Unknown test framework: could not find xunit.dll (v1) or xunit.execution.*.dll (v2) in C:\Users\hosch\Documents\Visual Studio 2015\Projects\roslyn\Binaries\Debug

I was working on this before the last release, and when I opened it after pulling all the latest changes, I had to clear and re-restore my nuget packages. Is this part of the problem?

@Hosch250
Copy link
Contributor Author

Hosch250 commented May 8, 2016

I got the tests working, @CyrusNajmabadi, don't worry about them (another 'restore.cmd' did the trick). A couple of them are failing, so I'll fix those and update the PR later (probably tomorrow). Feel free to rebase things any time, unless you would rather have me create a new branch off Future and move my changes over.

@CyrusNajmabadi
Copy link
Member

Ok. I've rebased and pushed. You will have to close and open a new PR against future. Thanks!

@Hosch250 Hosch250 closed this May 8, 2016
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.

Use Braces Analyzer/Code Fix