Skip to content

Fix OOB src project configurations #48318

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Feb 18, 2021
Merged

Fix OOB src project configurations #48318

merged 6 commits into from
Feb 18, 2021

Conversation

@ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Feb 15, 2021

Fixes #48248
Fixes #48306
Fixes #48307

@ViktorHofer ViktorHofer self-assigned this Feb 15, 2021
@ghost
Copy link

ghost commented Feb 15, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #48248
Fixes #48306
Fixes #48307

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Extensions-DependencyInjection
Milestone: -

@ghost
Copy link

ghost commented Feb 16, 2021

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #48248
Fixes #48306
Fixes #48307

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Extensions-DependencyInjection, area-Infrastructure-libraries
Milestone: -

safern
safern approved these changes Feb 16, 2021
View reviewed changes
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

LGTM

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Feb 16, 2021

@maryamariyan I don't think this is an infra issue per-se but I'm ok with using that label for the sake of just having a single area.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Feb 16, 2021

I talked with @ericstj about this offline and dropping a tfm that already shipped isn't considered a breaking change as long as the nearest compatible asset that is picked up instead isn't noticeably different (ie AssemblyVersion).

Based on that I dropped the net5.0 asset in Microsoft.Extensions.DependencyInjection which unintentionally shipped and downgraded the net5.0 asset in Microsoft.Extensions.Options to netcoreapp3.1 which was the tfm previously shipped before the project was migrated to dotnet/runtime. Spoke with @maryamariyan about this offline as well.

joperezr
joperezr approved these changes Feb 16, 2021
View reviewed changes
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.

LGTM thanks for fixing

@maryamariyan
Copy link
Contributor

maryamariyan commented Feb 17, 2021

@maryamariyan I don't think this is an infra issue per-se but I'm ok with using that label for the sake of just having a single area.

I was debating between area-Meta (because of multiple projects and it fixing different issues), area-Infra* (because of PR title), or just keeping DI (diff mostly in DI). Sure I'll move back,

@ghost
Copy link

ghost commented Feb 17, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #48248
Fixes #48306
Fixes #48307

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Extensions-DependencyInjection, area-Infrastructure-libraries
Milestone: -

@eerhardt eerhardt mentioned this pull request Feb 17, 2021
Closed
eerhardt
eerhardt approved these changes Feb 17, 2021
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for making these projects better while you were here, @ViktorHofer!

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Feb 17, 2021

Failure is #30056

@ViktorHofer ViktorHofer merged commit 225b700 into dotnet:master Feb 18, 2021
@ViktorHofer ViktorHofer deleted the FixPkgConfigurations branch February 18, 2021 14:38
@ghost ghost locked as resolved and limited conversation to collaborators Mar 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@marek-safar marek-safar marek-safar left review comments

@eerhardt eerhardt eerhardt approved these changes

@ericstj ericstj ericstj left review comments

@tarekgh tarekgh tarekgh left review comments

@safern safern safern approved these changes

@joperezr joperezr joperezr approved these changes

@maryamariyan maryamariyan Awaiting requested review from maryamariyan

Assignees

@ViktorHofer ViktorHofer

Labels

area-Extensions-DependencyInjection

Projects

None yet

Milestone

No milestone

8 participants

@ViktorHofer @maryamariyan @marek-safar @eerhardt @ericstj @tarekgh @joperezr @safern