use shared mergeup logic #23
Conversation
| @@ -111,7 +111,6 @@ class Consts | |||
| 'silverstripe-security-extensions', | |||
| // 'silverstripe-upgrader', | |||
| 'silverstripe-versionfeed', // not in commercially supported list, though is in cwp | |||
| 'sspak', | |||
There was a problem hiding this comment.
sspak was messing things up. It'd be removed when we do #3 in the future anyway.
| $off = $offset ? "{$op}{$offset}" : ''; | ||
| $off = $offset ? "&{$offset}" : ''; |
There was a problem hiding this comment.
This was resulting in queries like https://api.github.com/repos/silverstripe/gha-dispatch-ci/tags?per_page=100?page=1 (note the double ?)
There was a problem hiding this comment.
We need to be able to fetch files so we can get composer.json info
| // get branches | ||
| $json = $requester->fetch("/repos/$account/$repo?paginate=0", '', $account, $repo, $refetch); | ||
| $defaultBranch = $json->root->default_branch; | ||
| $repoData = MetaData::getMetaDataForRepository("$account/$repo"); | ||
| $composerJson = $requester->fetchFile($account, $repo, $defaultBranch, 'composer.json', $refetch); | ||
| $branchesJson = $requester->fetch("/repos/$account/$repo/branches", '', $account, $repo, $refetch)->root ?? []; | ||
| $allRepoBranches = array_map(fn($x) => $x->name, $branchesJson); | ||
| $tagsJson = $requester->fetch("/repos/$account/$repo/tags", '', $account, $repo, $refetch)->root ?? []; | ||
| $allRepoTags = array_map(fn($x) => $x->name, $tagsJson); | ||
| $branches = BranchLogic::getBranchesForMergeUp("$account/$repo", $repoData, $defaultBranch, $allRepoTags, $allRepoBranches, $composerJson); |
There was a problem hiding this comment.
This changes how we get the branches - we're now using the shared logic which is based on what branches and tags exist, as well as mapping branches to CMS major releases. See related PRs.
There was a problem hiding this comment.
Everything below this is about how those branches are displayed, which needed to be updated to correctly display these branches in the correct columns, matching to the correct CMS releases and whether it's a patch branch, etc.
| $columns = [ | ||
| $colPrefix . ' NextMin Branch', | ||
| $colPrefix . ' ToNextMin status', | ||
| $colPrefix . ' NextPat Branch', | ||
| $colPrefix . ' FromPrvMin status', | ||
| $colPrefix . ' PrvMin Branch', | ||
| ]; |
There was a problem hiding this comment.
I updated the columns to be more immediately representative of what is in that column, with less mental load to translate them.
Before:
After:
composer.json
Outdated
| "repositories": { | ||
| "silverstripe/supported-modules": { | ||
| "type": "vcs", | ||
| "url": "[email protected]:creative-commoners/supported-modules.git" | ||
| } | ||
| }, |
There was a problem hiding this comment.
This is required for now for testing - but once silverstripe/supported-modules#28 is merged and added to packagist I'll remove the fork and run composer update so we have an up-to-date composer.lock.
GuySartorelli
changed the title
GuySartorelli
force-pushed
the
pulls/main/use-shared-mergeup-logic
branch
from
8cf1514 to
45c3dc1
Compare
GuySartorelli
force-pushed
the
pulls/main/use-shared-mergeup-logic
branch
from
45c3dc1 to
448b137
Compare
composer.json
Outdated
| "repositories": { | ||
| "silverstripe/supported-modules": { | ||
| "type": "vcs", | ||
| "url": "[email protected]:creative-commoners/supported-modules.git" |
There was a problem hiding this comment.
Done, now that the repo is in packagist.
GuySartorelli
force-pushed
the
pulls/main/use-shared-mergeup-logic
branch
from
448b137 to
442804b
Compare
Note that the empty columns do still need to be in the logic, because there is some overlap in support levels where those columns will be used.
For now I've opted to keep the columns in the markup so we can get this merged and have accurate information displaying - we can open a separate card to hide the columns when they're empty if it bothers people enough to spend dev time on,
Issues