Skip to content
This repository has been archived by the owner on Jan 16, 2020. It is now read-only.

Add travis template #16

Merged
merged 5 commits into from
Aug 2, 2017
Merged

Add travis template #16

merged 5 commits into from
Aug 2, 2017

Conversation

geerteltink
Copy link
Member

See #15.

This PR adds a template for Travis CI configuration to attempt to have a central location for templates and communication about possible future changes.

Currently when updating a project the way to have an up-to-date .travis.yml file is looking it up in the latest released package and copy the changes from there. This makes the example config available in a central place.

Copy link
Member

@michalbundyra michalbundyra left a comment

Choose a reason for hiding this comment

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

Not sure but maybe we should include in that template also variables and commands to deploy documentation.

- if [[ $TEST_COVERAGE == 'true' ]]; then travis_retry composer require --dev $COMPOSER_ARGS $COVERAGE_DEPS ; fi
- stty cols 120
- export COLUMNS=120
- composer show
Copy link
Member

Choose a reason for hiding this comment

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

I would go here with:

- COLUMNS=120 composer show

instead of two above lines

Copy link
Member

Choose a reason for hiding this comment

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

Do we need both the stty cols 120 and export COLUMNS lines?

  • If only the stty command works, remove the export line.
  • If the export works, remove the stty line, and combine the export and the composer show lines as @webimpress suggests.

cache:
directories:
- $HOME/.composer/cache
- $HOME/.local
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need here vendor and zf-mkdoc-theme?

global:
- COMPOSER_ARGS="--no-interaction"
- COVERAGE_DEPS="satooshi/php-coveralls"
- LEGACY_DEPS="phpunit/phpunit"
Copy link
Member

Choose a reason for hiding this comment

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

What about other variables (for docs deploy)?

Copy link
Member

Choose a reason for hiding this comment

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

Since this is for new packages, I think we can ignore the LEGACY_DEPS situation entirely.

- php: 5.6
env:
- DEPS=locked
- TEST_COVERAGE=true
Copy link
Member

Choose a reason for hiding this comment

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

What about:

- DEPLOY_DOCS="$(if [[ $TRAVIS_BRANCH == 'master' && $TRAVIS_PULL_REQUEST == 'false' ]]; then echo -n 'true' ; else echo -n 'false' ; fi)"

Copy link
Member

@froschdesign froschdesign Jul 13, 2017

Choose a reason for hiding this comment

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

Correct, this is missing!

@weierophinney
Copy link
Member

Maybe we should include in that template also variables and commands to deploy documentation.

I'm working on having zfbot build the docs. Since the bot gets triggered for build status events, it can trigger doc builds when it detects a successful build on the master branch. Doing this means we can remove such integration out of the Travis config - which has a nice side effect of preventing token exposure (our previous token was exposed a couple months ago; github revoked it for us when it was detected). It will also make triggering new builds when templates change far easier.

@geerteltink
Copy link
Member Author

I'm working on having zfbot build the docs. Since the bot gets triggered for build status events, it can trigger doc builds when it detects a successful build on the master branch.

Exactly why I didn't include it. And more reason to have a centralized template as others didn't know about this yet.

@froschdesign
Copy link
Member

Exactly why I didn't include it.

Exactly why these boilerplates so important, because all others didn't know this!

Was used for docs generation?
@geerteltink
Copy link
Member Author

geerteltink commented Jul 16, 2017

I'm not so happy with these 2 lines:

env:
  global:
    - LEGACY_DEPS="phpunit/phpunit"

install:
  - if [[ $TRAVIS_PHP_VERSION =~ ^5.6 ]]; then travis_retry composer update $COMPOSER_ARGS --with-dependencies $LEGACY_DEPS ; fi

They are not reusable. Can't we write it like this:

matrix:
  include:
    - php: 5.6
      env:
        - DEPS=lowest
        - LEGACY_DEPS="phpunit/phpunit"

install:
  - if [[ $LEGACY_DEPS != "" ]]; then travis_retry composer update $COMPOSER_ARGS --with-dependencies $LEGACY_DEPS ; fi

This way we can reuse it for any php version where needed.

@froschdesign
Copy link
Member

@xtreamwayz

This way we can reuse it for any php version where needed.

Makes sense.

@michalbundyra
Copy link
Member

I think we will need to set then LEGACY_DEPS only on locked build.

Copy link
Member

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

Since this targets new packages, we can eliminate even more from the boilerplate: we don't need anything related to PHP 5.6 compat, and we can remove the 7.0 build targets as well.

global:
- COMPOSER_ARGS="--no-interaction"
- COVERAGE_DEPS="satooshi/php-coveralls"
- LEGACY_DEPS="phpunit/phpunit"
Copy link
Member

Choose a reason for hiding this comment

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

Since this is for new packages, I think we can ignore the LEGACY_DEPS situation entirely.


matrix:
include:
- php: 5.6
Copy link
Member

Choose a reason for hiding this comment

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

For new packages, we only need 7.1 and nightly; we can remove the 5.6 and 7.0 targets (though you'll have to move the TEST_COVERAGE env stuff to the 7.1 locked env).


install:
- travis_retry composer install $COMPOSER_ARGS
- if [[ $TRAVIS_PHP_VERSION =~ ^5.6 ]]; then travis_retry composer update $COMPOSER_ARGS --with-dependencies $LEGACY_DEPS ; fi
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed when you remove the 5.6 targets.

- if [[ $TEST_COVERAGE == 'true' ]]; then travis_retry composer require --dev $COMPOSER_ARGS $COVERAGE_DEPS ; fi
- stty cols 120
- export COLUMNS=120
- composer show
Copy link
Member

Choose a reason for hiding this comment

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

Do we need both the stty cols 120 and export COLUMNS lines?

  • If only the stty command works, remove the export line.
  • If the export works, remove the stty line, and combine the export and the composer show lines as @webimpress suggests.

@geerteltink
Copy link
Member Author

  • COLUMNS=120 composer show -> only shows package names
  • export COLUMNS=120 composer show -> doesn't show anything
  • export COLUMNS=120 && composer show -> only shows package names
  • stty cols 120 - > works
  • stty cols 120 && composer show -> works

For tests and travis composer show output see xtreamwayz/xtreamwayz.com#20

…iptions

COLUMNS=120 doesn't seem to do anything.
- php: 7.1
env:
- DEPS=latest
- php: nightly
Copy link
Member

Choose a reason for hiding this comment

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

Last change, I promise! However, I discovered today that we can now use the alias 7.2! They'll be updating it to track the betas, RCs, and then stable releases. I tested it on the zend-psr7bridge package, and it works brilliantly!

Until it's stable it should allow failures.
@weierophinney weierophinney merged commit ee1191a into zendframework:master Aug 2, 2017
weierophinney added a commit that referenced this pull request Aug 2, 2017
weierophinney added a commit that referenced this pull request Aug 2, 2017
@weierophinney
Copy link
Member

Thanks, @xtreamwayz!

@geerteltink geerteltink deleted the feature/travis-template branch August 3, 2017 06:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants