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

FIX Do not use recipe-plugin or vendor-plugin to work out cms major #82

Merged

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Feb 22, 2024

Issue silverstripe/.github#199

recipe-plugin and vendor-plugin ^2 both work in CMS 5 + 6, so cannot be used to determine dynamically work the CMS major version of installer any more.

It caused 5.x-dev to be used instead of 6.x-dev on https://github.com/tractorcow-farm/silverstripe-fluent/actions/runs/8011520958/job/21884953244?pr=827

Note the bit in the release process where we update this is from involves copying .cow.pat.json and running php hardcoded.php - this is used to add to the INSTALLER_TO_REPO_MINOR_VERSIONS const. vendor-plugin and recipe-plugin will still appear for this in this and that's fine

However that isn't used to workout the CMS major version of installer to use in dynamic e.g. major dev branch with no minor branches, instead it's harcoded version out which version of installer to use for a specific module minor branch. This is different from the dynamic approach which is being disabled in this PR

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Won't affect tests for those repos directly either since recipe-plugin has no tests and vendor-plugin explicitly doesn't need installer.

@GuySartorelli GuySartorelli merged commit ff51f00 into silverstripe:1.12 Feb 22, 2024
4 checks passed
@GuySartorelli GuySartorelli deleted the pulls/1.12/tmp-fluent-8 branch February 22, 2024 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants