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

[REFACTOR] Use WP-CLI via docker image rather than composer dep. #139

Merged
merged 2 commits into from
Jan 10, 2022

Conversation

jenkoian
Copy link
Contributor

Installing wp-cli via composer is fine, but it means we're at the mercy
of our other dependencies. i.e. we could hit a dependency version
conflict which could mean we couldn't install wp-cli or that other
library.

This uses the official docker image as part of our app image in much the
same way as we install composer itself.

Unfortunately (afaik) dependabot is unable to handle images within the
COPY command in docker, so we will need to keep the image up to date
ourselves.

Similarly, this commit also bumps the composer image we use to use the
latest version at time of writing (2.2.3).

Installing wp-cli via composer is fine, but it means we're at the mercy
of our other dependencies. i.e. we could hit a dependency version
conflict which could mean we couldn't install wp-cli or that other
library.

This uses the official docker image as part of our app image in much the
same way as we install composer itself.

Unfortunately (afaik) dependabot is unable to handle images within the
`COPY` command in docker, so we will need to keep the image up to date
ourselves.

Similarly, this commit also bumps the composer image we use to use the
latest version at time of writing (2.2.3).
@andrewjt71
Copy link

Minor: typo in 2nd commit The scaffold command is still required via compsoer

@andrewjt71
Copy link

Question:

Installing wp-cli via composer is fine, but it means we're at the mercy
of our other dependencies

I understand the reasoning behind this - in a nutshell, wp-cli is treated as an external lib which happens to be in php - it doesn't need to be compatible with our application code as it calls our application code externally like phpcs etc. So bringing it in via composer may raise conflicts which we don't care about.

However, seeing as we are now bringing wp-cli in via composer anyway (for the scaffolding command), doesn't this overhead still apply?

@jenkoian
Copy link
Contributor Author

However, seeing as we are now bringing wp-cli in via composer anyway (for the scaffolding command), doesn't this overhead still apply?

We are only bringing in the scaffold-command now as we extend it for our custom scaffolder. However, as this depends on wp-cli/wp-cli we bring that in also.

This is still a lower overhead to what was before which was bringing in wp-cli/wp-cli-bundle which I believe is what the docker image uses. We could also make the scaffold dep a suggestion if we wanted, further reducing the overhead.

So I guess by decoupling wp-cli from our application we have more options (I'm struggling to quantify but it also just feels cleaner).

Hope this helps clarify?

@andrewjt71
Copy link

This is still a lower overhead to what was before which was bringing in wp-cli/wp-cli-bundle

Thanks @jenkoian, this is the piece of the puzzle I was missing

The scaffold command is still required via composer as we extend it for
our custom scaffolder for mu-plugins.

The i18n-command is marked as a suggestion as it is only needed if a
site requires i18n and uses twig to make use of our make pot twig
command.
@jenkoian jenkoian merged commit e1152d0 into main Jan 10, 2022
@jenkoian jenkoian deleted the wp-cli-via-docker branch January 10, 2022 17:05
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.

3 participants