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

[WIP] New build tool #299

Closed
wants to merge 14 commits into from
Closed

[WIP] New build tool #299

wants to merge 14 commits into from

Conversation

mderriey
Copy link
Collaborator

@mderriey mderriey commented Sep 23, 2022

Fixes #298

This is my first attempt at trying to come up with something that fits the bill.
Mostly to get something in front of you and get feedback.

For now, I've created different workflows to make it easier for me to test things; in the future, we could look at consolidating them and including some logic within the workflow itself.
They're mostly the same, the things that change are:

  • The trigger: a pull request is opened, a pull request is merged (push), a tag is created (either for the main Carter package, or the Newtonsoft one)
  • Which package we push, and where it's pushed: this is done through the arguments we pass to the push.sh script

Here's the list of actions I took on the repo, and what would happen:

Action taken Result Link to workflow
Opened a PR Carter prerelease package pushed to feedz https://github.com/mderriey/Carter/actions/runs/3113410773/jobs/5048006852#step:6:1
Merged a PR Carter prerelease package pushed to NuGet https://github.com/mderriey/Carter/actions/runs/3113457288/jobs/5048111598#step:6:1
Created the 6.1.3 tag Carter release package pushed to NuGet https://github.com/mderriey/Carter/actions/runs/3113464170/jobs/5048127102#step:6:1
Carted the newtonsoft-6.0.0 tag Newtonsoft release package pushed to NuGet https://github.com/mderriey/Carter/actions/runs/3113483201/jobs/5048168976#step:6:1

on:
push:
branches:
- defaultinterface
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will change to main but I wanted to build on top of your defaultinterface branch where you had already moved Newtonsoft into its own project.

on:
push:
tags-ignore:
- 'newtonsoft-*'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This makes sure that it doesn't run for when we want to release a new version of the Newtonsoft package

Copy link
Member

Choose a reason for hiding this comment

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

Is that looking for a tag containing the term "newtonsoft"

Copy link
Collaborator Author

@mderriey mderriey Sep 26, 2022

Choose a reason for hiding this comment

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

Apologies, that should have read "for when we want to push a release of the Carter package".

And you're correct, this is ignoring tags that start with newtonsoft-

Copy link
Member

Choose a reason for hiding this comment

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

Thing is tags will probably just be v7.0.0 for example

Copy link
Collaborator Author

@mderriey mderriey Sep 26, 2022

Choose a reason for hiding this comment

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

I used different tags as I thought you wanted to be able to have different versions of the Carter and the Carter.ResponseNegotiators.Newtonsoft packages while the source stays in the same repo.

"Regular" tags, like v7.0.0, would release Carter, while newtonsoft-v7.0.0 would release the Carter.ResponseNegotiators.Newtonsoft package.

Did I make a wrong assumption thinking you might want to keep their versioning separate?
My thinking was that if we keep them in sync, new versions of Carter.ResponseNegotiators.Newtonsoft could be released without any changes in them.

on:
push:
tags:
- 'newtonsoft-*'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This makes sure that it runs only for when we want to release the Newtonsoft package.

push.sh Outdated
Comment on lines 4 to 9
TARGET_PACKAGE=$1
if [ "$TARGET_PACKAGE" = "carter" ]; then
TARGET_PACKAGE_PATH="./src/Carter/**/*.nupkg"
elif [ "$TARGET_PACKAGE" = "newtonsoft" ]; then
TARGET_PACKAGE_PATH="./src/Carter.ResponseNegotiators.Newtonsoft/**/*.nupkg"
fi
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not a Bash expert by any means.

This is the way I found to target either the main Carter or the Newtonsoft package,

@mderriey
Copy link
Collaborator Author

@jchannon would you mind taking a look and letting me know if that sort of fits what you had in mind?

Very keen for feedback and happy to make changes to adjust things.

Cheers.

@jchannon
Copy link
Member

Looks good, the only thing I wouldnt want is a push to nuget on a PR merge. For pre-releases I'd probably tag the repo as 7.0.0-beta1 for example

on:
push:
tags-ignore:
- 'newtonsoft-*'
Copy link
Member

Choose a reason for hiding this comment

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

Is that looking for a tag containing the term "newtonsoft"

- name: Build Reason
run: echo ${{github.ref}} and ${{github.event_name}}
- name: Build with dotnet
run: find . -path ./template -prune -false -o -name '*.csproj' -execdir dotnet build --configuration Release --nologo {} \;
Copy link
Member

Choose a reason for hiding this comment

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

I might have to change this or move the newtonsoft stuff out of the main repo otherwise this will find the csproj.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suppose it depends on how split out you want them to be. Right now it'll find all csprojs and build them; if that's an issue, we could look at using path filters in the GitHub Actions workflow to only build projects we're interested in.

I find it overkill, that project doesn't add much overhead now, but it's up to you.

@mderriey
Copy link
Collaborator Author

Looks good, the only thing I wouldnt want is a push to nuget on a PR merge. For pre-releases I'd probably tag the repo as 7.0.0-beta1 for example

OK, got it, I'll push an update for this.

@jchannon
Copy link
Member

jchannon commented Sep 26, 2022 via email

@mderriey
Copy link
Collaborator Author

No you were right, I just couldn't envisage how we'd do two separate tag approaches :)

Cool 👍

I am wondering if I move it out into its own repo whether that might be clearer however I'd still somehow want to bump those external packages to use the latest carter when we release a new carter version.

I think there are several options here:

  1. In the same workflow as the one where we release the Carter package, we can find/replace references to the Carter with the latest version; this works for things that are in the same repository.
  2. We could also imagine a separate workflow that queries NuGet for the latest Carter version, update package references, then run some kind of tests; this could work for things in the same repo and in different repositories.

Whether to move it in its own repository is up to you, I'd say.
If you don't foresee many other projects with a different release cycle, keeping it in here doesn't add too much "engineering" overhead in my opinion.
If/when we hit issues, we can always move it.
Maybe some folks at NDC will have opinions based on how painful it was for them 😉


For the template project, one thing I did in the past that worked fairly well was the following:

  • The CarterTemplate project would have a project reference to the Carter project — the benefit of this is that you get immediate feedback if you make an API change that affects the template project.
  • At release time, swap the project reference for a package reference (let's say 7.0.0 if that's what we're releasing).
  • Then pack and push the template project.

This way, you always keep an updated template project with the latest Carter version.

@jchannon
Copy link
Member

I mean we already have a couple of external repos that depend on carter as a package reference but have not been kept up to date really because of the complexities here and me being bothered to work out to do it best :) So if we did move the newtonsoft package out then we'd have 3 or 4 repos that need the package refence updated. Whilst you say we could query nuget, how would you do that, a timer or I wonder if there is a cross repo trigger eg/Carter gets tagged and can then somehow notify other repos.

Regarding the template I also came up with that idea but never got around to doing it, if we did swap the references I'd also want to do that automatically as I already manually change it once Carter is released and I don't like that :)

@mderriey
Copy link
Collaborator Author

Whilst you say we could query nuget, how would you do that, a timer or I wonder if there is a cross repo trigger eg/Carter gets tagged and can then somehow notify other repos.

I'm not aware of cross-repo way of triggering workflows.

My initial idea was manually triggering the workflow run (I didn't know there were other repos with package references to Carter), but scheduled runs could also work.

Regarding the template I also came up with that idea but never got around to doing it, if we did swap the references I'd also want to do that automatically as I already manually change it once Carter is released and I don't like that :)

I wasn't clear enough, apologies. Yes, the swapping was done automatically when I implemented that at work a while ago.

@jchannon
Copy link
Member

A manually triggered workflow that did a find and replace, commit and asked for a tag and then pushed and released to nuget could be very handy indeed!

@mderriey
Copy link
Collaborator Author

OK, we can have a look at that in the future.

How would you like to go forward with this PR? Do you want to wait for .NET 7 to be out and your changes to be merged to main, or should we try and get something merged soon-ish and adapt when the split-out Newtonsoft project gets merged to main?

Remove the publishing of a prerelease package when a PR is merged to `main` as per #299 (comment).
@mderriey
Copy link
Collaborator Author

@jchannon friendly bump about #299 (comment)

@jchannon
Copy link
Member

jchannon commented Sep 30, 2022 via email

@jchannon
Copy link
Member

jchannon commented Oct 1, 2022

As your PR is going to be merged into my branch which has all the .NET 7 changes on it I wonder if we can get it all working in your branch? I just approved a thing in GH Actions so any changes made in the workflow files should build here 🤞

@mderriey
Copy link
Collaborator Author

mderriey commented Oct 3, 2022

I've made a few changes to test out the real thing, but I believe it will need to be tested on your repo as I'm fairly certain I won't have access to your repo's secrets, like the feedz.io API key.

@mderriey
Copy link
Collaborator Author

mderriey commented Oct 5, 2022

It now says that the workflow is awaiting approval, so I suppose you need to do something on your end.

To test the whole thing, you'll need to add:

  • The feedz API key as FEEDZ_KEY, which you should already have.
  • The NuGet API key as NUGET_KEY.

The scenarios to test those would be:

  • Open a PR to trigger a prerelease package pushed to feedz.io (this PR should do the trick)
  • Create a tag, like X.X.X to trigger a release package pushed to NuGet.
  • Create a tag, like newtonsoft-X.X.X to trigger a Newtonsoft release package pushed to NuGet.

It's a bit awkward testing this as it will push packages to NuGet; can you unlist them afterwards? Or maybe you could create tags with versions lower than the current one so that consumers don't get asked to update?

You also mentioned earlier that you would create tags with a leading v, like v7.0.0; if you still want to go this way, I'll need to make a slight change to let MinVer know that the tags contain a prefix.

@jchannon
Copy link
Member

jchannon commented Oct 5, 2022

Ok, approved the workflow.

Tags don't seem to contain the prefix on the repo, I just checked.

@mderriey
Copy link
Collaborator Author

mderriey commented Oct 5, 2022

Ok, approved the workflow.

Sweet, thanks.

The run failed, supposedly because the workflow didn't have access to the FEEDZ_KEY secret, see https://github.com/CarterCommunity/Carter/actions/runs/3174168115/jobs/5201477628#step:6:8.

Maybe you need to enable forks having access to the secrets? It's a bit sketchy, unless you can do this on a case-by-case basis. Otherwise, we could look at making me a temporary contributor to this repo and I open a PR from a separate branch in this repo?

Tags don't seem to contain the prefix on the repo, I just checked.

OK, so you want to keep it this way, with no prefix?

@jchannon
Copy link
Member

jchannon commented Oct 5, 2022

Just given you write access! 😄
See if that helps, if not I can bump the access.

Yeah no prefix required

@mderriey mderriey mentioned this pull request Oct 5, 2022
@mderriey
Copy link
Collaborator Author

mderriey commented Oct 5, 2022

Just given you write access! 😄

Sweet, thanks, opened #300 and closing this PR.

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.

2 participants