-
Notifications
You must be signed in to change notification settings - Fork 11
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 Don't set up database etc if not needed #98
FIX Don't set up database etc if not needed #98
Conversation
.github/workflows/ci.yml
Outdated
|
||
if [[ NEEDS_FULL_SETUP == "false" ]]; then | ||
echo "skipping .env, database, and other full site setup steps" | ||
exit 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of existing which may mess up the rest of the job, just move the block of code inside the conditional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
18f7589
to
ddfd055
Compare
.github/workflows/ci.yml
Outdated
# composer install does actually remove the artifact directory | ||
mkdir artifacts | ||
|
||
if [[ NEEDS_FULL_SETUP == "false" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if [[ NEEDS_FULL_SETUP == "false" ]]; then | |
if [[ $NEEDS_FULL_SETUP == 'false' ]]; then |
Need a $
for the variable :-)
Using single quotes just to copy style above where a conditional check was done if [[ $SUCCESSFULLY_REQUIRED == 'false' ]]; then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The single/double quotes isn't consistent everywhere in this file (I copied double quotes from somewhere in here too) but I have no issues with that change.
Done.
ddfd055
to
a279c16
Compare
@GuySartorelli Once this has auto-tagged could you manually re-trigger a recipe + a regular module + something without installer just to double check things still work as expected |
Requires silverstripe/gha-generate-matrix#77 to be merged and tagged first.
Issue