Skip to content

Conversation

@eerhardt
Copy link
Member

No description provided.

@ghost
Copy link

ghost commented May 12, 2020

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

Copy link
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

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

Thanks for fixing

"AssemblyVersionInPackageVersion": {
"3.1.4.0": "5.0.0",
"3.1.3.0": "3.1.3",
"3.1.4.0": "3.1.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

dont we increment the last part in the servicing releases ?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe but this is just reacting to a package serviced in release/3.1 which was not yet part of libraries so may use different versioning

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’m not sure I understand the question.

This library was not part of corefx. This is the first release it is being built using the “corefx/libraries” packaging infrastructure.

Copy link
Member

Choose a reason for hiding this comment

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

He is saying that usually in servicing we only increment the last part of the assembly version, so package 3.1.4 should have had assembly version 3.1.3.1 instead of 3.1.4.0, but that wasn’t a decision made on this PR, this is just reacting to what already happened in servicing

Copy link
Contributor

Choose a reason for hiding this comment

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

yes i should have been more clearer, i meant to ask if the versioning scheme used here is on purpose or just a mistake in packaging (nothing wrong with this pr)

Copy link
Member

Choose a reason for hiding this comment

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

@Anipik keep in mind that at least in release/3.1 this package is being built on a different repo (coresetup) which means that they were using a slightly different infrastructure and were possibly not following the same versioning guidelines we do in corefx. Now we have moved it to runtime it should start following conventions starting from 5.0 release

@stephentoub
Copy link
Member

Can we merge this to unblock PRs?

@eerhardt
Copy link
Member Author

Can we merge this to unblock PRs?

Nothing should be blocked on this. My previous change fixed those. #36285 This is just a follow up to that.

@stephentoub
Copy link
Member

stephentoub commented May 13, 2020

Ah, ok, then we probably need to close and re-open some PRs to pick up your previous change. Thanks.

@joperezr
Copy link
Member

As @eerhardt suggests PRs shouldn’t be block by this, that said I’ll go ahead and merge as we don’t have anything else to address here

@joperezr joperezr merged commit b1baf3f into dotnet:master May 13, 2020
@eerhardt eerhardt deleted the FixDependencyModelPackageIndex branch May 13, 2020 14:22
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants