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 Use installer for silverstripe/serve and silverstripe/behat-exten… #80

Merged

Conversation

emteknetnz
Copy link
Member

Issue silverstripe/.github#188

silverstripe/serve and silverstripe/behat-extension used to get silverstripe/installer installed, now they don't and their CI stopped working

job_creator.php Outdated
Comment on lines 72 to 74
if (!isset($json->type) || !in_array($json->type, $silverstripeRepoTypes)) {
return '';
Copy link
Member

Choose a reason for hiding this comment

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

This condition is the one we want to skip, not the file_exists condition above. We should still look for silverstripe-related dependencies to guess the installer version.

Copy link
Member

Choose a reason for hiding this comment

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

Do these repos not also need dev/build run? If they do, we need to update needs_full_setup.
If they don't need that, then we should just add installer as an explicit dev dependencies for the relevant repositories (as mentioned in the description of the PR that introduced this condition)

Whenever we add a new repository that isn't a module/recipe/theme, we have to update a constant in order to tell the matrix generator not to add installer for it.

Instead of doing that, the generator just shouldn't try to add installer for those. Anything that isn't a recipe, a module, or a theme but which needs installer for its tests to run should explicitly add installer as a dev dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've moved the !in_array($this->repoName, FORCE_INSTALLER_UNLOCKEDSTEPPED_REPOS) to here.

End result is that serve + behat-extension will still get installer dynamically added as a dep in CI

explicit dev dependencies for the relevant repositories

But installer is not a real dev dependency. I also don't want to touch either serve + behat-extension because they're both fine, issue is just CI

@emteknetnz emteknetnz force-pushed the pulls/1.12/behat-serve branch from 501096b to 9fa6a4c Compare January 31, 2024 21:39
@emteknetnz emteknetnz force-pushed the pulls/1.12/behat-serve branch from 9fa6a4c to d200dc4 Compare January 31, 2024 21:40
@GuySartorelli GuySartorelli merged commit f166409 into silverstripe:1.12 Jan 31, 2024
4 checks passed
@GuySartorelli GuySartorelli deleted the pulls/1.12/behat-serve branch January 31, 2024 21:50
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