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

Merge-ups - logic needs to match gha-merge-ups #17

Closed
emteknetnz opened this issue Mar 4, 2024 · 6 comments
Closed

Merge-ups - logic needs to match gha-merge-ups #17

emteknetnz opened this issue Mar 4, 2024 · 6 comments
Assignees

Comments

@emteknetnz
Copy link
Member

emteknetnz commented Mar 4, 2024

Rhino by default shows too many columns, for instance as of CMS 5.1 for framework it will show:

  • 6.x-dev
  • 5.x-dev
  • 5.1.x-dev
  • 5.0.x-dev (incorrect as it's unsupported)
  • 4.x-dev
  • 4.13.x-dev
  • 4.12.x-dev (incorrect as it's unsupported)

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:

  • 6.x-dev
  • 5.x-dev
  • 5.2.x-dev (pre-stable)
  • 5.1.x-dev
  • 4.x-dev
  • 4.13.x-dev

We should extract the logic from gha-merge-up to determine which branches should be shown

Acceptance criteria

  • The merge up logic in Rhino to select branches matches the merge up logic used by our GitHub actions.

Note

  • The idea was floated of a joint library or API where we can house this logic, so we don't have duplicated code

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:

  • update the relevant confluence docs regarding the updated data locations.
  • deploy Rhino
  • Make sure the gha repos get tagged appropriately
@emteknetnz emteknetnz changed the title Merge-ups - logic needs to march gha-merge-ups Merge-ups - logic needs to match gha-merge-ups Mar 4, 2024
@GuySartorelli GuySartorelli self-assigned this Apr 22, 2024
@GuySartorelli
Copy link
Member

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 master which isn't a number and therefore throws an exception. The merge-ups logic is simply incompatible with default branches that don't match our branching strategy.

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:

  1. All logic about which branches on which repos belong to which major release line for Silverstripe CMS should be moved into the supported-modules repo.
  2. The supported-modules repo should only have a single branch which holds all of that information for all relevant branches.
  3. The supported-modules repo becomes a package accessible via composer, with a well-defined PHP API - but the JSON files remain more-or-less as they are (albeit combined into a single branch) for the non-PHP projects which rely on it. I think right now that's only Elvis, but I'd have to double check.
  4. Generate matrix, mergeups, and Rhino all pull in the supported-modules repo for logic regarding which branches they should be operating on, and what each of those branches represent.

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

@emteknetnz
Copy link
Member Author

emteknetnz commented Apr 23, 2024

All logic about which branches on which repos belong to which major release line for Silverstripe CMS should be moved into the supported-modules repo.

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?)

The supported-modules repo should only have a single branch which holds all of that information for all relevant branches.

OK, so we something like "branches": ["4" => ["3"], "5" => ["4"], "6" => ["5"]] to denote CMS version => repo branches for that CMS version ?

An alternative approach would be to put all of that logic in a new separate repo

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

@GuySartorelli
Copy link
Member

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?)

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.

OK, so we something like "branches": ["4" => ["3"], "5" => ["4"], "6" => ["5"]] to denote CMS version => repo branches for that CMS version?

Yeah something along those lines probably

We have so many repos already :-) We probably don't need more

Yup I agree. Just figured we should explicitly reject that option rather than not mention it.

@emteknetnz
Copy link
Member Author

Have merged the first PR, assigning back to Guy

@emteknetnz
Copy link
Member Author

PRs merged, assigning back to Guy

@GuySartorelli
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants