CarterCommunity / Carter

Carter is framework that is a thin layer of extension methods and functionality over ASP.NET Core allowing code to be more explicit and most importantly more enjoyable.
MIT License
2.11k stars 174 forks source link

New build tool #300

Closed mderriey closed 1 year ago

mderriey commented 1 year ago

Supersedes #299.

Same code, the difference is that this PR comes from a local branch in this repo (not my fork), to make it easier to access the repository secrets.

mderriey commented 1 year ago

Cool, the push to feedz worked, see https://github.com/CarterCommunity/Carter/actions/runs/3188686481.

I'm wondering if we can test the other workflows by creating tags on my branch. If my understanding is correct, the workflows should run from the defaultinterface-workflow branch...

Would you mind trying this out?

jchannon commented 1 year ago

i think you should be able to tag it with your access?

On Wed, 5 Oct 2022 at 10:22, Mickaël Derriey @.***> wrote:

Cool, the push to feedz worked, see https://github.com/CarterCommunity/Carter/actions/runs/3188686481.

I'm wondering if we can test the other workflows by creating tags on my branch. If my understanding is correct, the workflows should run from the defaultinterface-workflow branch...

Would you mind trying this out?

— Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/pull/300#issuecomment-1268177474, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJTUHSHLM3AJVMCGZY3WBVCEPANCNFSM6AAAAAAQ5MP2XA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

mderriey commented 1 year ago

OK, I'll give it a go.

Do we have a NuGet API key set as a repo secret with the NUGET_KEY name?

mderriey commented 1 year ago

I'll try without it and we'll see what happens.

jchannon commented 1 year ago

I just added one. I always used to just push via my local machine so it wasn't there

mderriey commented 1 year ago

I just added one. I always used to just push via my local machine so it wasn't there

Thanks!

I pushed a tag, and the workflow failed with the following error

error: Response status code does not indicate success: 403 (The specified API key is invalid, has expired, or does not have permission to access the specified package.).

Does that make sense to you? Maybe the Carter package is linked with another NuGet account than the one the API key is associated with?

jchannon commented 1 year ago

Looks like it expired. Have just updated the NUGET_KEY secret in Github so hopefully should now work

On Thu, 6 Oct 2022 at 07:34, Mickaël Derriey @.***> wrote:

I just added one. I always used to just push via my local machine so it wasn't there

Thanks!

I pushed a tag, and the workflow failed https://github.com/CarterCommunity/Carter/actions/runs/3188775222/jobs/5201819877#step:7:12 with the following error

error: Response status code does not indicate success: 403 (The specified API key is invalid, has expired, or does not have permission to access the specified package.).

Does that make sense to you? Maybe the Carter package is linked with another NuGet account than the one the API key is associated with?

— Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/pull/300#issuecomment-1269412001, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJQJ4M6PYYOBC7TROB3WBZXGZANCNFSM6AAAAAAQ5MP2XA . You are receiving this because you commented.Message ID: @.***>

mderriey commented 1 year ago

@jchannon OK, looks like this worked, thank you.

Tag I created Workflow Resulting NuGet package
7.0.0-alpha.1 https://github.com/CarterCommunity/Carter/actions/runs/3198520376 https://www.nuget.org/packages/Carter/7.0.0-alpha.1
newtonsoft-7.0.0-alpha.1 https://github.com/CarterCommunity/Carter/actions/runs/3198534365 https://www.nuget.org/packages/Carter.ResponseNegotiators.Newtonsoft/7.0.0-alpha.1

Do you think that's good enough as a first step to be merged, or do you want more refinement?

Things we need to do first:

mderriey commented 1 year ago

@jchannon

Friendly bump, please let me know if there are other things you want done as part of this PR. Cheers.

jchannon commented 1 year ago

Looks good! If you need to add the tests back in then please do and I’ll merge it into my branch :)

On Tue, 11 Oct 2022 at 11:24, Mickaël Derriey @.***> wrote:

@jchannon https://github.com/jchannon

Friendly bump, please let me know if there are other things you want done as part of this PR. Cheers.

— Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/pull/300#issuecomment-1274295608, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJT45B5BUFF2QAO3T3TWCUP4PANCNFSM6AAAAAAQ5MP2XA . You are receiving this because you were mentioned.Message ID: @.***>

jchannon commented 1 year ago

I've unlisted them!

If you add the tests back in that'd be great :)

I'm happy to merge if you want. What flows do we get if we merge now and are there any things outstanding if we merge now?

On Thu, 6 Oct 2022 at 16:48, Mickaël Derriey @.***> wrote:

@jchannon https://github.com/jchannon OK, looks like this worked, thank you. Tag I created Workflow Resulting NuGet package 7.0.0-alpha.1 https://github.com/CarterCommunity/Carter/actions/runs/3198520376 https://www.nuget.org/packages/Carter/7.0.0-alpha.1 newtonsoft-7.0.0-alpha.1 https://github.com/CarterCommunity/Carter/actions/runs/3198534365 https://www.nuget.org/packages/Carter.ResponseNegotiators.Newtonsoft/7.0.0-alpha.1

Do you think that's good enough as a first step to be merged, or do you want more refinement?

Things I need to do first:

  • Add tests back in the build, I removed them because they weren't passing
  • Do you want to unlist the newly-pushed packages so that people don't accidentally pull them, given the move to .NET 7 is not complete yet?
  • Feel free to add to this list so we can track the remaining tasks before we can build

— Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/pull/300#issuecomment-1270303875, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJXLVASIGOAEDWJZZK3WB3YDXANCNFSM6AAAAAAQ5MP2XA . You are receiving this because you were mentioned.Message ID: @.***>

mderriey commented 1 year ago

If you add the tests back in that'd be great :)

Done, and I fixed the failing tests in https://github.com/CarterCommunity/Carter/pull/300/commits/42e15aed363f46251dc0ab3003816ca2e7e9fb4c. Please take a look to ensure that this is the right fix 😉

I've also added an action that publishes test results as part of a workflow run check. See how it looks here: https://github.com/CarterCommunity/Carter/actions/runs/3229749031/jobs/5287405935 If you don't like it, happy to remove it.

What flows do we get if we merge now?

  1. Automated publish of a prerelease Carter package to feedz.io on PR builds.
  2. Automated publish of the Carter package to NuGet.org on tags that don't start with newtonsoft-, i.e. like 7.0.0, or 7.0.0-preview.1.
  3. Automated publish of a stable Carter.ResponseNegotiators.Newtonsoft on tags that do start with newtonsoft-, i.e. like newtonsoft-7.0.0 or newtonsoft-7.0.0-preview.1.

are there any things outstanding if we merge now?

We talked about a couple more things, but IMO they can be done as part of separate PRs. Off the top of my head, I can think about:

jchannon commented 1 year ago

Awesome stuff @mderriey !! 🎉

Thanks for the help, much appreciated!

Now onto the auto publishing/bumping if you're up for it 😄