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

Add SourceLink and GitVersioning #50

Merged
merged 14 commits into from
Jun 7, 2020
Merged

Add SourceLink and GitVersioning #50

merged 14 commits into from
Jun 7, 2020

Conversation

ssa3512
Copy link
Contributor

@ssa3512 ssa3512 commented Jun 6, 2020

Resolves

Resolves #42, #35

Description

This PR adds a Directory.Build.props, Directory.Build.targets and version.json in the root of /src to add sourcelink, gitversioning and deterministic builds to MLOps.NET

Documentation on GitVersioning: https://github.com/dotnet/Nerdbank.GitVersioning
Documentation on Deterministic builds is here: https://github.com/clairernovotny/DeterministicBuilds

TODO:

  • Publish PR packages - might need @aslotte to set this up (requires a package feed and keys) edit: publishing as build/CI artifact
  • Fix deterministic portion (nuget package explorer shows untracked sources warning and non-deterministic

@ssa3512
Copy link
Contributor Author

ssa3512 commented Jun 6, 2020

@aslotte I enabled <GenerateDocumentationFile> with this so the nuget package contain an xml doc file (for intellisense etc) - this generates a bunch of CS1591 warnings for all public members that do not have doc comments. Do you prefer these be suppressed or would you want to keep them so they can be addressed in a separate issue?

@ssa3512 ssa3512 marked this pull request as draft June 6, 2020 20:11
@aslotte
Copy link
Owner

aslotte commented Jun 6, 2020

That's awesome @ssa3512, thank you for looking into this, let me open a new issue for comments on public properties/methods.

@aslotte
Copy link
Owner

aslotte commented Jun 6, 2020

Created #52 to track this. Let me know if you have anything else to add there.

@aslotte
Copy link
Owner

aslotte commented Jun 6, 2020

Added interface summary comments to #54 that was just closed

@aslotte
Copy link
Owner

aslotte commented Jun 6, 2020

What can I do next before we can merge this? In your experience, does it make sense to publish PR nuget packages to something like GitHub nuget packages, or just leave them as artifacts?

@ssa3512
Copy link
Contributor Author

ssa3512 commented Jun 6, 2020

What can I do next before we can merge this? In your experience, does it make sense to publish PR nuget packages to something like GitHub nuget packages, or just leave them as artifacts?

I don't know that it really makes sense to publish PR packages to a feed as they aren't really meant for consumption. From what I have seen it makes sense to have CI packages go to a separate feed (myget/azure artifacts/github/etc) and then you can publish beta, rc, and release to nuget.org.

I would recommend a second job using jobs.<job.id>.if for publishing CI pacakges and probably a manually triggered job that is locked down to only you for pushing to nuget.org

@ssa3512
Copy link
Contributor Author

ssa3512 commented Jun 7, 2020

I am having trouble getting packages to push to github package registry correctly, even in my own test repository. I will have to dig into it some further. It looks like there are some caveats around using GPR nuget and github actions.

@aslotte
Copy link
Owner

aslotte commented Jun 7, 2020

Thanks for looking into this @ssa3512. I haven't worked much with CI/CD on nuget packages so I appreciate your input. I'll try to get up to speed in the next few days.

@ssa3512 ssa3512 marked this pull request as ready for review June 7, 2020 17:44
@ssa3512
Copy link
Contributor Author

ssa3512 commented Jun 7, 2020

This should be ready now - will always create packages and publish as an artifact, but will only push to your repository's nuget feed for CI on master.

You can see an example of the published packages here: https://github.com/ssa3512/MLOps.NET/packages/258569

If you open with Nuget Package Explorer they light up green for sourcelink and deterministic

image

To change the version, you just update src/version.json. The third digit in the package is always git-height since the last version change.

To create a release package you can create a branch vx.y where x & y are major and minor version and update the version.json to drop the pre-release tag. The dotnet global tool nbgv will also do this with the prepare-release command.

@aslotte aslotte changed the title [WIP] Add SourceLink and GitVersioning Add SourceLink and GitVersioning Jun 7, 2020
@aslotte
Copy link
Owner

aslotte commented Jun 7, 2020

@ssa3512 this is great, thank you for the explanation as well. I've learned a lot personally by reviewing this contribution. Let me get this merged and set up my access token in secrets.

Copy link
Owner

@aslotte aslotte left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thank you for your contribution!

@aslotte aslotte merged commit bda1bf3 into aslotte:master Jun 7, 2020
@ssa3512
Copy link
Contributor Author

ssa3512 commented Jun 7, 2020

@aslotte I don't think you need to set up an access token - because this runs in the repository GITHUB_TOKEN is automatically available. https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token#about-the-github_token-secret

@aslotte
Copy link
Owner

aslotte commented Jun 7, 2020

I just saw, it's magic :)

@aslotte
Copy link
Owner

aslotte commented Jun 7, 2020

Question @ssa3512 - we would expect a package to be pushed on merge to master during the CI build right? Just wondering as I don't see any packages so far. The artifacts seem to be created, but they are missing the git suffix

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.

Add CI/CD pipeline for NuGet packaging and deployment
3 participants