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

[9.0.2xx] sln migrate, sln list, sln add #45408

Merged
merged 8 commits into from
Dec 18, 2024

Conversation

edvilme
Copy link
Member

@edvilme edvilme commented Dec 10, 2024

Manual backport of #44419 , #44537 , #44661 #44570, #45130, and #45366 to release/9.0.2xx

Fixes #40913.

@kasperk81
Copy link
Contributor

@edvilme source-build-ext update for 9.0.2x requires kasperk81@c713864

eng/Version.Details.xml Outdated Show resolved Hide resolved
@edvilme edvilme force-pushed the 9.0.2xx-edvilme-slnx-add branch from 72eaa1d to ff11407 Compare December 12, 2024 16:39
@marcpopMSFT
Copy link
Member

@baronfel we should get this reviewed tomorrow and merge so that it can get into the next VS insertion for preview 3.

@marcpopMSFT marcpopMSFT requested a review from Forgind December 18, 2024 00:40
Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

Content-wise, I glanced through the commits, and they looked equivalent to what was merged as I'd expected, but it's essentially a whole new set of commits, which I don't like too much. I would've hoped for at least some of them to not be in need of resolving merge conflicts, especially if you tweak it to look as the next commit would expect in earlier commits...but I probably shouldn't nitpick on that right now. You can make that a little easier too by making sure all your commits are in order, which you mostly did except one that got moved out of place 😉

@kasperk81
Copy link
Contributor

this should include #45160 and "fix #40913" in description so the downstream consumers get some time to enable slnx in 9.0.200 timeframe

@baronfel
Copy link
Member

Since the only failing checks are due to the macOS arm64 leg, let's merge on read and get this in! @marcpopMSFT care to do the honors?

@marcpopMSFT marcpopMSFT merged commit e0fb73c into dotnet:release/9.0.2xx Dec 18, 2024
28 of 31 checks passed
@marcpopMSFT
Copy link
Member

Content-wise, I glanced through the commits, and they looked equivalent to what was merged as I'd expected, but it's essentially a whole new set of commits, which I don't like too much. I would've hoped for at least some of them to not be in need of resolving merge conflicts, especially if you tweak it to look as the next commit would expect in earlier commits...but I probably shouldn't nitpick on that right now. You can make that a little easier too by making sure all your commits are in order, which you mostly did except one that got moved out of place 😉

We can give pointers when he's back in the office as there are some strategies for avoiding merge conflicts especially in our repo (target the live branch and let codeflow take to main as an example).

@marcpopMSFT
Copy link
Member

We'll get remove in once it passes checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants