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

Populate RepositoryBranch #1248

Merged
merged 6 commits into from
Jul 26, 2024
Merged

Conversation

MattKotsenas
Copy link
Member

@MattKotsenas MattKotsenas commented Jul 11, 2024

Fixes #1243.

Adds BranchName as an output of the LocateRepository task and also adds it to the SourceRoot for the main repository. The locate task maps the BranchName output task to SourceBranchName in the .targets file to follow the naming convention. The branch is the full symbolic-ref name and not the short name to avoid ambiguity in source control systems like GitHub which use the refs/pull namespace for Pull Requests.

This change doesn't add branch name to submodules, as fundamentally submodules only track commits, and the main branch will fully specify any submodule-related metadata.

The corresponding change to map this value into RepositoryBranch as part of NuGet pack is @ NuGet/NuGet.Client#5923.

Open issues in the PR:

  1. Some of the CloudHostedProvidersTests.cs specifically use the inbox version of the Git tasks, and as such they can't assert the new functionality. I'm unsure if that's by design and I should not worry about that scenario, or if the tests should be changed?
  2. Open to name changes anywhere; I went with the simple name, but happy to rename if there's something more appropriate.

@MattKotsenas
Copy link
Member Author

/cc @tmat & @baronfel for review and questions. Thanks so much!

@baronfel
Copy link
Member

I haven't looked in detail, but to answer the question about where the package metadata mapping should go I'd say a followup PR to NuGet would be the appropriate place - they already have the mappings for the other automatic package properties.

@MattKotsenas
Copy link
Member Author

I haven't looked in detail, but to answer the question about where the package metadata mapping should go I'd say a followup PR to NuGet would be the appropriate place - they already have the mappings for the other automatic package properties.

OK, I can get started on that. Separately but relatedly, do you know the flow for updates? I'm new in this area, but to "ship" this feature end-to-end it sounds like we'd need at least:

  1. Update SourceLink package
  2. Update NuGet client package
  3. SDK update to pull in new versions of 1 + 2

Is this a manual process, or are these picked up automatically? Any info greatly appreciated. Thanks!

@baronfel
Copy link
Member

There's automated code flow of both SourceLink and NuGet targets into the SDK now - if you're working against main branch for all of the repos then you're targeting .NET 9 at this point, which feels fine to me.

@MattKotsenas
Copy link
Member Author

Created a draft in NuGet.Client: NuGet/NuGet.Client#5923.

@tmat, would you mind taking a look, or letting me know who can review? Happy to make any changes requested. Thanks!

@tmat
Copy link
Member

tmat commented Jul 22, 2024

I'll take a look.

@tmat
Copy link
Member

tmat commented Jul 26, 2024

@MattKotsenas Looks great! Thanks for the PR. Just minor update to the comments and it's good to go.

@MattKotsenas
Copy link
Member Author

Thanks @tmat! I believe I fixed all your comments. Would you mind taking another look?

@MattKotsenas MattKotsenas requested a review from tmat July 26, 2024 20:18
@tmat tmat merged commit 3bef25f into dotnet:main Jul 26, 2024
10 checks passed
@MattKotsenas MattKotsenas deleted the feature/default-branch branch July 26, 2024 21:13
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.

Populate RepositoryBranch automatically
3 participants