Skip to content

Update Visual Studio Version to 17.0#6361

Merged
ladipro merged 7 commits intodotnet:mainfrom
Forgind:17.0
May 6, 2021
Merged

Update Visual Studio Version to 17.0#6361
ladipro merged 7 commits intodotnet:mainfrom
Forgind:17.0

Conversation

@Forgind
Copy link
Copy Markdown
Contributor

@Forgind Forgind commented Apr 21, 2021

Fixes #6360

Context

Branding update didn't hit Visual Studio version

Changes Made

Fixed that

Testing

None

Notes

I messed up on my commit history, but the changes are right. Shouldn't matter if we squash it in.

@Forgind Forgind changed the title 17.0 Update Visual Studio Version to 17.0 Apr 21, 2021
Copy link
Copy Markdown
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

We'll also need a bunch of the stuff from #6336. Let's merge that in, fi to 17.0, and add the new changes.

@Forgind
Copy link
Copy Markdown
Contributor Author

Forgind commented Apr 21, 2021

Yeah—I saw there was a lot that depended on "Current" version. So to clarify, no action item for this PR until the first two steps are complete, and then this will be good to squash?

@rainersigwald
Copy link
Copy Markdown
Member

no action item for this PR until the first two steps are complete,

correct

and then this will be good to squash?

No; we'll want to add a couple of 17.0 things on top of Kirill's PR.

Comment thread src/Tasks/Microsoft.VisualStudioVersion.v17.Common.props Outdated
@rainersigwald
Copy link
Copy Markdown
Member

@Forgind can you merge main to 17, then rebase or remerge this? Then we'll need to add a couple more things.

@Forgind
Copy link
Copy Markdown
Contributor Author

Forgind commented Apr 22, 2021

I merged main into this, but I don't think I have permission to push directly to the 17.0 branch, so I'd have to add it via PR.

I made a couple tweaks that looked necessary from 6336. What did I miss?

(Other than building, now fixed.)

@rainersigwald
Copy link
Copy Markdown
Member

I merged main into this, but I don't think I have permission to push directly to the 17.0 branch, so I'd have to add it via PR.

Yes, please. As is the diff is very confusing, and the commit history will be a bit of a mess (when we eventually come along and do 18.0 I'd like to have as few commits as possible so we can look back).

@rainersigwald
Copy link
Copy Markdown
Member

I'm still finding this diff intensely confusing. Mind squashing this branch into a better form?

git checkout 17.0
Already on '17.0'
Your branch is up to date with 'Forgind/17.0'.git rev-parse HEAD
fb1d9659863d5e77053d86a23fe9ca06e4575013git reset --hard upstream/vs17.0git merge --squash fb1d9659863d5e77053d86a23fe9ca06e4575013
Automatic merge went well; stopped before committing as requested
Squash commit -- not updating HEADgit commit -am "Additional 17.0 branding/internal updates"
[17.0 8519d595c6] Additional 17.0 branding/internal updates
 10 files changed, 58 insertions(+), 16 deletions(-)
 create mode 100644 src/Tasks/Microsoft.VisualStudioVersion.v17.Common.props

@Forgind
Copy link
Copy Markdown
Contributor Author

Forgind commented Apr 23, 2021

I'd neglected to do any of that since main merged into vs17.0. It was also nice to have the commands written out for me so I didn't have to think about it 🙂

Copy link
Copy Markdown
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

At last it's understandable! I think GitHub got confused about what the merge-base should be because vs17.0 was moved to a place slightly ahead of your branch. I don't know why that was confusing though . . .

image

Comment thread src/Utilities/ToolLocationHelper.cs Outdated
Co-authored-by: Rainer Sigwald <raines@microsoft.com>
@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Apr 23, 2021
Copy link
Copy Markdown
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Please change the VSIX to contain the new VS file:

file source=$(X86BinPath)Microsoft.VisualStudioVersion.v16.Common.props

@ladipro ladipro mentioned this pull request May 4, 2021
@ladipro
Copy link
Copy Markdown
Member

ladipro commented May 5, 2021

@Forgind after addressing feedback please consider repointing to main which is now becoming our 17.0 development branch.

@Forgind Forgind changed the base branch from vs17.0 to main May 5, 2021 16:15
Copy link
Copy Markdown
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

@marcpopMSFT this is the PR I'd really like in VS 17.0 ASAP.

@rainersigwald
Copy link
Copy Markdown
Member

Looks like missing this is breaking some internal C++ tests. @ladipro I'll include you on the mail. We should take this ASAP and get an insertion going so we can let the QB figure out when to take it.

@ladipro ladipro merged commit d07c47a into dotnet:main May 6, 2021
@Forgind Forgind deleted the 17.0 branch May 6, 2021 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

17.0 branding update incomplete

5 participants