Skip to content

"Deconstruct variable declaration" should not introduce errors#25282

Merged
jinujoseph merged 2 commits intodotnet:masterfrom
Neme12:useDeconstruction
Mar 23, 2018
Merged

"Deconstruct variable declaration" should not introduce errors#25282
jinujoseph merged 2 commits intodotnet:masterfrom
Neme12:useDeconstruction

Conversation

@Neme12
Copy link
Copy Markdown
Contributor

@Neme12 Neme12 commented Mar 6, 2018

fixes #25260

@Neme12 Neme12 requested a review from a team as a code owner March 6, 2018 23:15
@jcouv jcouv added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Mar 11, 2018
jcouv
jcouv reviewed Mar 17, 2018
View reviewed changes
src/EditorFeatures/CSharpTest/UseDeconstruction/UseDeconstructionTests.cs
{
void M()
{
(string name, int age) [|person|] = default;
Copy link
Copy Markdown
Member

@jcouv jcouv Mar 17, 2018

Choose a reason for hiding this comment

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

Filed #25559

@jcouv jcouv added the Area-IDE label Mar 17, 2018
jcouv
jcouv approved these changes Mar 23, 2018
View reviewed changes
Copy link
Copy Markdown
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

@jcouv
Copy link
Copy Markdown
Member

jcouv commented Mar 23, 2018

@dotnet/roslyn-ide Please review and merge this community PR. Thanks

@jcouv jcouv added this to the 15.8 milestone Mar 23, 2018
@jcouv
Copy link
Copy Markdown
Member

jcouv commented Mar 23, 2018

@Pilchie Do we need some kind of ask-mode approval for master? If yes, could you approve?

@jinujoseph
Copy link
Copy Markdown
Contributor

jinujoseph commented Mar 23, 2018

@jcouv no master doesn't need ask mode approval.
I was waiting for test to complete

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Mar 23, 2018 •
edited
Loading

I don't know what's going on 😕 it says WorkItemAttribute can't be found, but it's building fine for me locally.

@jinujoseph
Copy link
Copy Markdown
Contributor

jinujoseph commented Mar 23, 2018 •
edited
Loading

confirmed with @jasonmalinowski , known issue ..merging this ..Thanks @Neme12

@jinujoseph jinujoseph merged commit 302866b into dotnet:master Mar 23, 2018
@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Mar 23, 2018 •
edited
Loading

Wait, it seems like a real issue, using Roslyn.Test.Utilities must have been removed from the file, even though it's still shown in the diff.

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Mar 23, 2018 •
edited
Loading

I'm afraid this is going to break the build :(

@Neme12 Neme12 mentioned this pull request Mar 23, 2018
Merged
@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Mar 23, 2018 •
edited
Loading

@jcouv @jinujoseph @jasonmalinowski I think this is going to break the build :-/ This was a real build issue, the using was removed in this PR #25643

I created a PR to fix it #25689
I'm really sorry :-/

@sharwell
Copy link
Copy Markdown
Contributor

sharwell commented Mar 23, 2018

@Neme12 No problem, thanks for the fix 😄

@jasonmalinowski
Copy link
Copy Markdown
Member

jasonmalinowski commented Mar 23, 2018

Don't worry, we award our build break squirrel to whoever hit the merge button, so @jinujoseph now gets to keep it company.

@jcouv
Copy link
Copy Markdown
Member

jcouv commented Mar 23, 2018

I've checked out the latest bits from master after this PR was merged and I'm indeed seeing the build break.

@jinujoseph
Copy link
Copy Markdown
Contributor

jinujoseph commented Mar 23, 2018

@Neme12 don't worry ...it was my mistake , i deserve the squirrel , thanks for the fix

@Neme12 Neme12 deleted the useDeconstruction branch April 6, 2018 06:52
@Neme12 Neme12 mentioned this pull request Apr 13, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcouv jcouv jcouv approved these changes

+2 more reviewers

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi approved these changes

@jinujoseph jinujoseph jinujoseph approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Milestone

15.8

Development

Successfully merging this pull request may close these issues.

"Deconstruct variable declaration" introduces error.

6 participants

@Neme12 @jcouv @jinujoseph @sharwell @jasonmalinowski @CyrusNajmabadi