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

nuget: fix PR missing commits in message when using private registry #5002

Merged
merged 9 commits into from
Apr 19, 2022

Conversation

jakecoffman
Copy link
Member

@jakecoffman jakecoffman commented Apr 14, 2022

Context

Dependabot PRs are missing the release notes and commits sections that we expect them to have when using a private NuGet registry like nuget.pkg.github.com.

This is because the GitHub NuGet registry doesn't support downloading .nuspec files currently, and this is the way we're finding the source repo of the project.

The .nuspec URL is part of the NuGet Server spec.

A solution

This PR adds a fallback to try to find the projectUrl or licenseUrl in the search data, which might contain the repo. The search URL is supported by GitHub Packages and the official NuGet server.

This will require users to enter a repo URL in the projectUrl, instead of the repositoryUrl field, since repositoryUrl isn't exposed in the search endpoint. The only way to get to that repositoryUrl seems to be in that .nuspec file.

Another possible solution is downloading the .nupkg and extracting the .nuspec, but I think that could have an effect on billing and that is probably undesirable. Also there's a risk the .nupkg is quite large since it contains binaries.

We could also ask GitHub Packages to implement that endpoint, this PR is meant as a stopgap or fallback when the nuspec is not available.

No tests yet: Wanted to get buy-in from the approach here.

@jakecoffman jakecoffman requested a review from a team as a code owner April 14, 2022 20:40
Copy link
Contributor

@mctofu mctofu left a comment

Choose a reason for hiding this comment

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

This approach makes sense to me! Thanks for the detailed explanation!

Agree that it's best to avoid downloading the .nupkg.

@jakecoffman
Copy link
Member Author

@dependabot/reviewers This is ready for review now.

Copy link
Contributor

@brrygrdn brrygrdn left a comment

Choose a reason for hiding this comment

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

👍🏻 I like this approach, :shipit:

@jakecoffman jakecoffman merged commit ae2a2a3 into main Apr 19, 2022
@jakecoffman jakecoffman deleted the jakecoffman/nuget-repo-from-private branch April 19, 2022 17:34
@brrygrdn brrygrdn restored the jakecoffman/nuget-repo-from-private branch April 25, 2022 14:07
@brrygrdn brrygrdn deleted the jakecoffman/nuget-repo-from-private branch April 25, 2022 14:07
brrygrdn added a commit that referenced this pull request Apr 25, 2022
…po-from-private"

This reverts commit ae2a2a3, reversing
changes made to 8851caa.
brrygrdn added a commit that referenced this pull request Apr 25, 2022
…po-from-private"

This reverts commit ae2a2a3, reversing
changes made to 8851caa.
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