Skip to content

branding update#6321

Merged
Forgind merged 3 commits intodotnet:vs17.0from
Forgind:17.0
Apr 5, 2021
Merged

branding update#6321
Forgind merged 3 commits intodotnet:vs17.0from
Forgind:17.0

Conversation

@Forgind
Copy link
Copy Markdown
Contributor

@Forgind Forgind commented Apr 2, 2021

Branding update for 17.0.

I skipped the readme changes, since I what we normally put would not be true at this point, since we haven't forked for 16.11 or even 16.10, and changes in main (we expect) will go into 16.10/11 still.

@olgaark, should I change ..\..\..\Microsoft\VC\v160\Microsoft.Build.CPPTasks.Common.dll to ..\..\..\Microsoft\VC\v170\Microsoft.Build.CPPTasks.Common.dll?

Are there any other changes that need to be made for a major version release?

@Forgind Forgind changed the base branch from main to vs17.0 April 2, 2021 17:25
@olgaark
Copy link
Copy Markdown
Contributor

olgaark commented Apr 2, 2021

No, please don't change v160 to v170 now - v170 does not exist yet (but will in Preview 2 and more msbuild.exe.config changes will be needed then)

Copy link
Copy Markdown
Member

@benvillalobos benvillalobos left a comment

Choose a reason for hiding this comment

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

Compared it to #6087, looks fine to me pending @olgaark 's approval.

Comment thread src/MSBuild/app.amd64.config Outdated
<assemblyIdentity name="Microsoft.Build.CPPTasks.Common" culture="neutral" publicKeyToken="b03f5f7f11d50a3a" />
<bindingRedirect oldVersion="16.0.0.0-16.10.0.0" newVersion="16.10.0.0" />
<codeBase version="16.10.0.0" href="..\..\..\Microsoft\VC\v160\Microsoft.Build.CPPTasks.Common.dll" />
<bindingRedirect oldVersion="16.0.0.0-17.0.0.0" newVersion="17.0.0.0" />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can app.config files read properties from the build? Or is it possible to define some variable that will update each app.config? (not a blocking question)

Copy link
Copy Markdown
Contributor

@olgaark olgaark Apr 2, 2021

Choose a reason for hiding this comment

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

You still need codebase, I believe
<codeBase version="17.0.0.0" href="..\..\..\Microsoft\VC\v160\Microsoft.Build.CPPTasks.Common.dll" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On the other hand, how about just removing this redirect? It won't be needed for 17.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, please remove Microsoft.Build.CPPTasks.Common from msbuild.exe.config at all. We don't want a redirect between major versions (and I plan to investigate if we can get rid of the redirect between minor versions as well)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we leave it but make it not do anything (that is, oldVersion="17.0.0.0-17.0.0.0") rather than removing it entirely? I'm thinking of when we start updating it, at which point it would be nicer to have the version already there and be able to just modify it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, please remove. My hope is that we won't need to add it again

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if you are removing it, you need to remove the whole segment

<dependentAssembly>
          <assemblyIdentity name="Microsoft.Build.CPPTasks.Common" culture="neutral" publicKeyToken="b03f5f7f11d50a3a" />
          <bindingRedirect oldVersion="16.0.0.0-17.0.0.0" newVersion="17.0.0.0" />
          <codeBase version="17.0.0.0" href="..\..\Microsoft\VC\v160\Microsoft.Build.CPPTasks.Common.dll" />
        </dependentAssembly>

@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 5, 2021
@Forgind Forgind merged commit efe639a into dotnet:vs17.0 Apr 5, 2021
@Forgind Forgind deleted the 17.0 branch April 5, 2021 19:34
@ladipro ladipro mentioned this pull request May 4, 2021
ladipro pushed a commit to ladipro/msbuild that referenced this pull request May 4, 2021
ladipro pushed a commit that referenced this pull request May 6, 2021
* branding update (#6321)

branding update

* Additional 17.0 branding/internal updates

* Update src/Utilities/ToolLocationHelper.cs

Co-authored-by: Rainer Sigwald <raines@microsoft.com>

* Update vsix

Co-authored-by: Rainer Sigwald <raines@microsoft.com>
Co-authored-by: Ben Villalobos <4691428+BenVillalobos@users.noreply.github.com>
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.

3 participants