Skip to content

Conversation

@FelicianoTech
Copy link

Hey,

Here is a first stab at a CircleCI config to test the doctor-command repo.

This is using CircleCI v2.1 config syntax and designed to be as close as possible to what's currently happening in the Travis CI builds.

Some immediate differences differences:

  • on the free plan, only 4 containers max will run in parallel
  • There isn't a PHP v5.4 job. CircleCI (because of upstream Docker Library/PHP) don't have recently built PHP 5.4 images. Honestly, PHP 5.4 went EOL over 3 years ago so this shouldn't be a problem.
  • I didn't implement the "sniff" job/commands.

Please let me know if there are any questions to what I did, my approach, or how something works.

Also, I was told that the CI process for the command repos seem to more or less be the same. If that's the case, once this config is completed, optimized, and vetted, it could be turned into a CircleCI Orb. This will allow the config to be updated in one location (the orb repo) and can be used in all of the command repos being built on CircleCI. Implementing DRY config.

type: string
default: "latest"
docker:
- image: circleci/php:<< parameters.php >>-browsers
Copy link
Member

Choose a reason for hiding this comment

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

Clever!

@schlessera
Copy link
Member

Looks like a great start already, @FelicianoTech, many thanks for that!

A few questions about the current state:

  • Missing PHP 5.4 is a real bummer, as we're currently committed to support environments down to PHP 5.4 for up to one year after WP has raised their minimum to PHP 5.6+ (which is planned to happen in April this year). Is there a way to somehow make this work nevertheless, even if it is not pretty and only a stop-gap until we raise the minimum? In case this is not possible, I'll have to look for a different way of running at least superficial tests against a PHP 5.4 system.
  • Why did you opt to skip the "sniff" job? Is it technically more difficult to pull this off (separate flows) or was it just to get going with an MVP?
  • The composer phpunit should be part of the testing, not the setup stage (before the composer behat one).
  • I think I enabled CircleCI for this repo now, so it should be triggered if you make a new push to this PR if I'm not mistaken.
  • Using a Circle Orb sounds awesome, that would solve a big maintenance hassle.

@FelicianoTech
Copy link
Author

  • PHP v5.4 - I don't know why WordPress still supports it. Even moving the minimum to v5.6 is also a horrible idea considering that that version of PHP is about to go EOL as well. I personally, strongly don't recommend supporting it. That being said, while CircleCI doesn't have a PHP v5.4 image (there's no circleci.php:5.4), upstream does (php:5.4).
  • Yes I skipped the sniff job simply to reduce the scope of this PR. I wanted to provide just enough for you all to get how the CircleCI build could work and then let you take it from there based on your needs.

I'll make one last push to this PR including the 5.4 image and moving the composer line you mentioned.

@FelicianoTech
Copy link
Author

@schlessera Builds for forked pull requests need to be turned on in the CircleCI Settings for this project.

@danielbachhuber
Copy link
Member

Builds for forked pull requests need to be turned on in the CircleCI Settings for this project.

I've enabled this.

@FelicianoTech
Copy link
Author

Sorry @danielbachhuber it looks like this was an old project readded so some settings are off.

"Enable build processing (preview)" needs to be turned on as well.

@danielbachhuber
Copy link
Member

No worries, enabled that as well.

@schlessera
Copy link
Member

I'm merging this as-is for now to be able to play with it outside of a PR. I'll do potential additional fixes in separate PRs.

Many thanks for the help with this, @FelicianoTech !

@schlessera schlessera merged commit fec8931 into wp-cli:master Mar 30, 2019
@schlessera schlessera added this to the 2.0.1 milestone Mar 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants