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.

Sign up for GitHub

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

Jump to bottom

Support record structs in complete statement command #52697

Merged
merged 3 commits into from
Apr 22, 2021
Merged

Support record structs in complete statement command #52697

merged 3 commits into from
Apr 22, 2021

Conversation

Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Apr 17, 2021 •
edited
Loading

Test plan: #51199

@Youssef1313 Youssef1313 requested a review from a team as a code owner April 17, 2021 17:03
@Youssef1313
Copy link
Member Author

Youssef1313 commented Apr 17, 2021 •
edited
Loading

Converting to draft since thia might not be needed if #52702 is done and merged. (Only one change wasn't needed after 52702 was merged)

@Youssef1313 Youssef1313 marked this pull request as draft April 17, 2021 20:24
@jinujoseph jinujoseph added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Apr 19, 2021
@Youssef1313 Youssef1313 marked this pull request as ready for review April 22, 2021 08:49
@Youssef1313
Copy link
Member Author

Closing and reopening due to CI flakiness

@Youssef1313 Youssef1313 reopened this Apr 22, 2021
@jcouv
Copy link
Member

jcouv commented Apr 22, 2021

Thanks for the fix. FYI, I am planning to merge the feature to features/compiler ASAP (PR #52671). When that is done, the features/record-structs branch will be closed and any PRs there will be retargeted (simple operation in GitHub). Sorry for the inconvenience.

jcouv
jcouv approved these changes Apr 22, 2021
View reviewed changes
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 3)

@Youssef1313
Copy link
Member Author

I'll retarget once #52671 is merged. Thanks for letting me know.

@CyrusNajmabadi CyrusNajmabadi merged commit bba92f9 into dotnet:features/record-structs Apr 22, 2021
@CyrusNajmabadi
Copy link
Member

Thanks!

@Youssef1313 Youssef1313 deleted the patch-17 branch April 22, 2021 16:57
@jcouv
Copy link
Member

jcouv commented Apr 22, 2021

@Youssef1313 No worry. This PR got merge first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi approved these changes

@jcouv jcouv jcouv approved these changes

Assignees

@CyrusNajmabadi CyrusNajmabadi

@jcouv jcouv

Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Youssef1313 @jcouv @CyrusNajmabadi @jinujoseph