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

Optimise build configuration #2062

Merged
merged 4 commits into from
Sep 24, 2019
Merged

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Sep 17, 2019

Q A
Type improvement
BC Break no
Fixed issues

Summary

  • Simplify usage of build steps to make this more consistent with the master branch
  • Fix wrong version number in 4.0 build stage. This resulted in the default MongoDB version (currently 4.0.7) to be used on travis-ci
  • Extract coverage to a separate build to speed up build times
  • Add builds against all MongoDB versions supported by the PHP driver

This makes the build files for 1.3.x and master more consistent, with master then using this version of the build file and just adding the new code quality stages.

@alcaeus alcaeus self-assigned this Sep 17, 2019
@alcaeus alcaeus requested a review from jmikola September 17, 2019 18:25
@alcaeus alcaeus force-pushed the optimise-build-config branch from e3d64aa to f71c795 Compare September 17, 2019 18:30
@alcaeus alcaeus added this to the 1.3.0 milestone Sep 18, 2019
@alcaeus alcaeus force-pushed the optimise-build-config branch from f71c795 to 5e2ea07 Compare September 23, 2019 06:53
@@ -16,32 +16,51 @@ env:
- ADAPTER_VERSION="^1.0.0"
- SERVER_DISTRO=ubuntu1604
- SERVER_VERSION=4.2.0
matrix:
- SERVER_VERSION="3.0.15"
- SERVER_VERSION="3.2.22"
Copy link
Member

Choose a reason for hiding this comment

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

3.0 and 3.2 are EOLed, is there any reason to test them?

Copy link
Member Author

Choose a reason for hiding this comment

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

The driver still supports them, which is why I added them to the pipeline. I wouldn't go out of my way to support them, but if we can verify that tests pass without additional work then I'm good for it

@alcaeus alcaeus force-pushed the optimise-build-config branch from 5e2ea07 to d43fcda Compare September 23, 2019 09:09
@alcaeus alcaeus changed the base branch from 1.3.x to master September 23, 2019 09:10
@alcaeus alcaeus changed the base branch from master to 1.3.x September 23, 2019 09:10
@alcaeus alcaeus force-pushed the optimise-build-config branch from d43fcda to 99aa219 Compare September 23, 2019 10:33
@alcaeus alcaeus force-pushed the optimise-build-config branch from 99aa219 to b6c64ad Compare September 23, 2019 10:36
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

One question regarding Xdebug, but LGTM.

php: 7.2
env: DEPLOYMENT=SHARDED_CLUSTER_RS
before_script:
- mv ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/xdebug.ini{.disabled,}
Copy link
Member

Choose a reason for hiding this comment

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

Is this a clever one-liner to rename the xdebug.ini.disabled INI file to xdebug.ini?

Would it make sense to add || echo "xdebug not available" here, as is done in before_install below?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, this expands to mv .../xdebug.ini.disabled .../xdebug.ini. In this case, there is no echo as the next line fails the build if xdebug was not found, providing a more detailed message. The mv in before_install disables the xdebug extension if present, printing an info message if it wasn't available to begin with (which is not a failure condition as we don't want it there for most builds).

@alcaeus alcaeus merged commit 41a9bc3 into doctrine:1.3.x Sep 24, 2019
@alcaeus alcaeus deleted the optimise-build-config branch September 24, 2019 07:13
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