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

Updating SignToolData to include the produced nuget packages #1860

Merged
merged 5 commits into from
Jan 19, 2018
Merged

Updating SignToolData to include the produced nuget packages #1860

merged 5 commits into from
Jan 19, 2018

Conversation

tannergooding
Copy link
Member

No description provided.

@tannergooding
Copy link
Member Author

FYI. @livarcocc

@livarcocc
Copy link
Contributor

Could we just cherry-pick the commit? All these merge commits are annoying.

@tannergooding
Copy link
Member Author

The merge commits will get created eventually (from the auto-merge bot).

"Microsoft.DotNet.PlatformAbstractions.dll",
"Microsoft.Extensions.DependencyModel.dll",
"Microsoft.Win32.Primitives.dll",
"netfx.force.conflicts.dll",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised that most of these are in our nupkg. They are supposed to be pulled in separately by the CLI from the corefx support package.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe most of them are in the tools directories.

Copy link
Member Author

Choose a reason for hiding this comment

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

(for netcoreapp)

Copy link
Contributor

Choose a reason for hiding this comment

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

Most of them are from Microsoft.NET.Build.Extensions\msbuildExtensions\Microsoft\Microsoft.NET.Build.Extensions, but those are excluded from the CLI ingestion here: https://github.com/dotnet/cli/blob/5c35438cfe1ac17fd4c748b8be6c2818d1238807/build/MSBuildExtensions.targets#L10-L17

We could (and probably should) exclude them from the .nupkg and not need to update these exclusions when the corefx support package changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

tools\netcoreapp2.0 has nothing but our assembly + satellites. Some of these are from tools/net46 and necessary, but not all of them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough, but that looks like it should be a separate work item, since this has been like this since before the infrastructure switch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. I filed #1865

@livarcocc
Copy link
Contributor

But 5 of them?

@tannergooding
Copy link
Member Author

Yes. 1 is created for each merge.

There is one merge locally (dotnet/release/2.0.0 into tagoo/release/2.1)
And one more when the PR is merged (tagoo/release/2.1 into dotnet/release/2.1)
Then one more for the next local merge (dotnet/release/2.1 into tagoo/master)
And one more when that PR is merged (tagoo/master into dotnet/master).

There are some ways to remove some of the merges and create a more "linear" history (such as telling GitHub to rebase and merge or squash and merge), but those end up losing some information and can potentially make other actions more difficult (the only downside is the additional, sometimes empty, merge commits bringing both branches of the tree together).

@tannergooding tannergooding merged commit 50c2444 into dotnet:master Jan 19, 2018
JL03-Yue pushed a commit that referenced this pull request Mar 19, 2024
…519.2 (#1860)

[main] Update dependencies from dotnet/arcade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants