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

Migrate to Central Package Management #16235

Merged
merged 10 commits into from
Jun 9, 2024
Merged

Migrate to Central Package Management #16235

merged 10 commits into from
Jun 9, 2024

Conversation

tmat
Copy link
Contributor

@tmat tmat commented Jun 4, 2024

Replace custom package management targets with NuGet Central Package Management.

Although the custom ApplyPackageManagement target introduced by #1777 works, it causes unnecessary build cache misses under certain circumstances. This in turn degrades Visual Studio build performance.

Copy link
Contributor

github-actions bot commented Jun 4, 2024

Thank you for submitting your first pull request, awesome! 🚀 If you haven't already, please take a moment to review our contribution guide. This guide provides helpful information to ensure your contribution aligns with our standards. A core team member will review your pull request.

If you like Orchard Core, please star our repo and join our community channels.

@Piedone
Copy link
Member

Piedone commented Jun 4, 2024

Thank you! Let us know when you're ready for a review.

@tmat tmat marked this pull request as ready for review June 5, 2024 16:01
@tmat
Copy link
Contributor Author

tmat commented Jun 5, 2024

@Piedone Ready now. The last commit reverts downgrade of Microsoft.Identity.Web to minimize impact of this change. Transitive pinning is preferred though and can be enabled as a follow up.

Copy link
Member

@Piedone Piedone left a comment

Choose a reason for hiding this comment

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

Otherwise looks good, thank you, but we'll check it together at the triage meeting tomorrow as well.

Copy link
Member

Choose a reason for hiding this comment

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

Add this file to the solution, to the build solution folder.

Copy link
Member

Choose a reason for hiding this comment

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

@tmat can you please address this item so we can merge this PR?

src/OrchardCore.Build/Dependencies.AspNetCore.props Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Jun 6, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

@Piedone
Copy link
Member

Piedone commented Jun 6, 2024

When you're done addressing all feedback of a review, please click "Re-request review" in the top-right corner for each reviewer when you're ready for another round of review, so they know that you're done.

@sebastienros
Copy link
Member

I thought this PR happened because we mentioned it during our meetings last week. But apparently not.

Copy link
Member

@MikeAlhayek MikeAlhayek left a comment

Choose a reason for hiding this comment

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

LGTM

@Piedone
Copy link
Member

Piedone commented Jun 6, 2024

Don't merge before #16235 (comment).

src/OrchardCore.Build/TargetFrameworks.props Outdated Show resolved Hide resolved
Directory.Packages.props Outdated Show resolved Hide resolved
@sebastienros
Copy link
Member

I moved the version to Directory.Packages.props. I hope this will work, I did it online directly.
Also updates the files in the solution as @Piedone suggested.

@MikeAlhayek MikeAlhayek merged commit 7f36f40 into OrchardCMS:main Jun 9, 2024
6 checks passed
Copy link
Contributor

github-actions bot commented Jun 9, 2024

Congratulations on your first PR merge! 🎉 Thank you for your contribution! We're looking forward to welcoming other contributions of yours in the future. @all-contributors please add @tmat for code.

If you like Orchard Core, please star our repo and join our community channels.

Copy link
Contributor

@github-actions[bot]

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

I've put up a pull request to add @tmat! 🎉

@tmat
Copy link
Contributor Author

tmat commented Jun 10, 2024

Thanks!

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.

5 participants