sbt
Feature/workflow support
@djspiewak I do not want flying to mars and then realising we just want to go to the moon. So could you take a look and tell me, if I am in the right space ship? 🚀 😃
This is a really nice start! Definitely headed in the right direction, though I have some aesthetic quibbles with file organization. :-P
Still wip ;)
I realised until now, you have the data model separated from the rendering logic. Would you like to keep this? Or you think my mixed approach is ok too?
As you can see, I copied some useful methods from GenerativePlugin. Do you have any suggestion where to put such code for usage in both files?
My gut feeling says, that GenerativePlugin is quite big.
Is working on this topic maybe a good place to rethink the current design and maybe split the file a bit? 🤔
For simplification, I plan not to implement branches-ignore or tags-ignore for events that would support it.
Since one can not mix it with branches and tags, but there is the negation syntax !<branchPattern>.
You can exclude tags and branches using the ! character. The order that you define patterns matters.
A matching negative pattern (prefixed with !) after a positive match will exclude the Git ref. A matching positive pattern after a negative match will include the Git ref again.
I'll add a part about it in the docs. See https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-using-positive-and-negative-patterns
@djspiewak I think the model should fit now, as well as the generation.
I tried to go with a minimal code change approach. I also don't think this changes should be breaking in any means, since the usage of Workflow in the old logic is not reachable from the public, if I see this correctly.
Imo the current state should be fine for an initial release of the feature.
Open questions:
-
githubWorkflowsettings like the Scala version, OSs or the definition for runs-on are the same for all jobs. Should that be customisable? - Should there be more validation? Cron expressions for the
Scheduletrigger event for example?
@djspiewak I am using this version currently in multiple projects and it works well so far. I changed to the way oses are compiled and added the GenerationTarget since I realised I don't want to have CI and Custom workflows.
But I think the current solution should be flexible enough for all needs.
@987Nabil does this support adding the workflow_dispatch event as a trigger?
@catostrophe For custom workflows yes. For the default CI job aka ci.yml no. push and pull_request are currently still hard coded for the ci job.
It is in this state, since the intention is not to break any old code.
@987Nabil that's exactly what I need - to tweak the default ci.yml to run on workflow_dispatch event. I wanted to add this as a small PR via a flag
@djspiewak what's the best way to introduce this feature? As a separate small PR or as a part of this one?
@catostrophe my suggestion would be: Merge this PR first, then do it separately afterward. I think this PR is quite big already. Even though I guess it might be a small change.
@djspiewak I just merged the main. And I am already heavily using it with a local build. Would be nice if you could review this and I could use an official build :)
@djspiewak I updated the PR with support for reusable workflows and merged in the master. If you need any kind of help, let me know. I would really love to merge this soonish 😉
@eed3si9n I am currently using a self build version that is based on this pr and since nothing happens for so long I was going to create a new lib. Which I actually don't like, because i think one lib of it's kind is more than enough. But since this is living now in the SBT org, I thought I ask you, if you are fine with me reviving this pr? If so, it would be great if you can comment on the general structure of the or, so i could then go on to fix/update it :slightly_smiling_face:
@987Nabil
But since this is living now in the SBT org, I thought I ask you, if you are fine with me reviving this pr? If so, it would be great if you can comment on the general structure of the or, so i could then go on to fix/update it 🙂
I am here as a backstop, but generally the maintainers are @sbt/sbt-github-actions team. If there are no breaking changes to the existing builds, I think the PR looks pretty good already. Maybe it should be better documented in README?
@987Nabil I can have a look at it, but to get the premise of the PR by custom workflow you mean the ability to generate a completely custom github action workflow flow with the various actions.
If so I can try to look at it during in a week (I have a conference to go to). In the meantime if you could rebase the PR to fix the current merge conflicts that would be great!
Also agree with @eed3si9n regarding the documentation
@eed3si9n sorry, I just saw SBT org and you changing something, so I assumed 😅
@mdedetrich basically it aims to be a scala DSL for actions/workflows. And yes, the goal is to not change any api but just extend it. I think the origin of the plugin was focused on OSS builds. But companies need maybe more complex CI/CD pipelines with more customizable jobs/steps and would benefit from having reusable code building blocks.
@987Nabil Thanks for the response, as mentioned before I will realistically look at it from next week Monday but if I can find some time I might start earlier.
I suspect that the reason behind the merge conflicts is the permissions feature at https://github.com/sbt/sbt-github-actions/pull/105 which I wrote and was consequently merged. On the surface I think it actually makes sense to incorporate that feature into your DSL when doing the rebase.
@mdedetrich there where also changes to the GitHub api. So I'd like to update anyway. I'll check on it today or maybe tomorrow.
@mdedetrich Sorry it might take longer than I expected. Maybe I can find some time today.
@mdedetrich I guess this is now in a reviewable state. The branch is up to date and I migrated the permissions. I simplified and generalized some code a bit. This comes with a little less beautiful workflows (see the missing empty lines), but is an overall benefit I guess, also in terms of maintainability. I am happy to add more docu, if you can give me a hint what you would expect. Should I add an example?
Also I did not check, if this state on the same feature level as the latest github api. But I guess it is ok to add stuff later if missing. The PR is already large.
@987Nabil Thanks, I will start looking at it in a few days. Note that some of the scripted tests are still failing