-
Notifications
You must be signed in to change notification settings - Fork 1
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
Merge-ups - logic needs to match gha-merge-ups #17
Comments
I got the Rhino logic updated locally to reflect how merge-ups is doing things only to realise that fails as soon as it hits a bringyourownideas repo, which have a default branch of This was a known problem, I just hadn't considered how it would affect Rhino. We have a few options here. One is to just not show those repos in Rhino, but that's worse than our current situation because then we'd have no idea when those need to be merged up - which right now has to be done manually so we need that visibility. Another, which I want to try, is to fix merge-ups to not care what the default branch is. There's logic in the generate matrix workflow which handles these scenarios and correctly identifies which branches of each repo belong to which major release. We also have a supported modules repo which holds similar information. I want to avoid having this information in 4 different places, which is what would happen if I just copy that logic/info into the merge-ups action and Rhino. My current thinking is as follows:
An alternative approach would be to put all of that logic in a new separate repo - I'd be open to that option, but we'll need to consider what the actual scope of the repo is so we know what sort of things should go there in the future. e.g. is it just a "GitHub Actions shared logic" repo (which other projects like Rhino can also use if they need to perform similar logic)? Or is it a "Silverstripe CMS branches and versions logic" repo, which can be used by GitHub actions but isn't necessarily specifically/exclusively for them? etc |
This is already a thing isn't it? https://github.com/silverstripe/supported-modules/blob/5/modules.json shows this already? Or are you saying move the PHP logic there? (it seems like you are?)
OK, so we something like
We have so many repos already :-) We probably don't need more. Given it that would be using the json file hosted in the repo it makes sense to just have it in supported-modules |
It has more limited information than is required for matrix generation, I assume - or else presumably we'd be using it there already. But also yes I'm proposing that relevant PHP logic is also moved there.
Yeah something along those lines probably
Yup I agree. Just figured we should explicitly reject that option rather than not mention it. |
Have merged the first PR, assigning back to Guy |
PRs merged, assigning back to Guy |
Rhino deployed and looking good. We've got a lot of modules without a workflow run with missing merge-ups, which was hard to tell before. I'll open a separate card for that. The two workflow repos have been tagged and work as expected on admin. |
Rhino by default shows too many columns, for instance as of CMS 5.1 for framework it will show:
It's incorrectly showing that 4.x-dev to 5.0.x-dev requires a merge-up. gha-merge-up won't touch 5.0.x-dev
However when we release the 5.2 beta, while that branch is pre-stable we'll need to see the following branches:
We should extract the logic from gha-merge-up to determine which branches should be shown
Acceptance criteria
Note
Follow-up cards created
PRs
NOTE These can all be reviewed now, but merge the supported-modules PR before anything else. Once that's merged, reassign to Guy to update the composer.json on the other two.
They're currently pulling in the fork so that we can see CI is green, and so that rhino can be easily stood up locally and tested.
AFTER MERGING
Some data which needs to be updated after minor releases has moved.
Some data which needs to be updated when creating a new repository has moved.
After merging, reassign to Guy so they can:
The text was updated successfully, but these errors were encountered: