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

Travis, 4.0 API fixes #216

Merged
merged 10 commits into from
Oct 9, 2017
Merged

Travis, 4.0 API fixes #216

merged 10 commits into from
Oct 9, 2017

Conversation

wilr
Copy link
Member

@wilr wilr commented Sep 12, 2017

Fixed #215 and fixed #217

.travis.yml Outdated
# Execute tests with coverage. Do this for a small
- "if [ \"$COVERAGE\" = \"1\" ]; then vendor/bin/phpunit --coverage-clover=coverage.clover comments/tests/; fi"
script:
- if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit; fi
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to pass the directory with the tests in

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah.. I was basing it off https://github.com/silverstripe/silverstripe-multiform/blob/master/.travis.yml#L30-L35 which doesn't seem to pass anything in unless I'm missing some flag

Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine to omit as long as it's defined in phpunit.xml.dist

- "if [ \"$COVERAGE\" = \"1\" ]; then mv coverage.clover ~/build/$TRAVIS_REPO_SLUG/; fi"
- cd ~/build/$TRAVIS_REPO_SLUG
- "if [ \"$COVERAGE\" = \"1\" ]; then travis_retry codecov; fi"
- if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi
Copy link
Contributor

Choose a reason for hiding this comment

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

With codecov there's a codecov.yml file that disables the PR comments which would be good to add

We'll also need to introduce the php-codesniffer composer dev requirement, and add minimum stability dev and prefer stable true to composer.json so this new style of Travis config will run.

I've started work on this here for reference: https://github.com/robbieaverill/silverstripe-comments/commits/pulls/3.0/fix-travis-config

devtool: (ENV !== 'production') ? 'source-map' : '',
module: moduleCSS(ENV, PATHS),
plugins: pluginCSS(ENV, PATHS),
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're not building JS at all then we can remove most of the npm dependencies right?

.travis.yml Outdated
env:
global:
- DB=MYSQL CORE_RELEASE=4
dist: precise
Copy link
Contributor

Choose a reason for hiding this comment

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

For SS4 we should definitely be trying to use trusty if possible since precise is being phased out

@wilr wilr changed the title Travis fix WIP: Travis, 4.0 API fixes Sep 13, 2017
dhensby
dhensby previously approved these changes Oct 9, 2017
@dhensby dhensby dismissed their stale review October 9, 2017 13:44

comments addressed

@robbieaverill robbieaverill changed the title WIP: Travis, 4.0 API fixes Travis, 4.0 API fixes Oct 9, 2017
@wilr wilr merged commit 8c43e09 into silverstripe:master Oct 9, 2017
@wilr wilr deleted the pulls/travis-fix branch October 9, 2017 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants