Remove Travis CI and add GitHub workflows CI #167
Conversation
unfulvio-godaddy
changed the title
composer.json
Outdated
| "config": { | ||
| "platform-check": false |
There was a problem hiding this comment.
Why was this added?
There was a problem hiding this comment.
sorry what I really meant was this: 82d4ec6
thanks for reminding me
| # OSX | ||
| **/.DS_Store | ||
|
|
||
| # IDEs | ||
| .project/ | ||
| .settings/ | ||
| .idea/ | ||
| *.code-workspace | ||
| *.sublime-project | ||
| *.sublime-workspace | ||
| *.iml | ||
| .vscode/ |
There was a problem hiding this comment.
I would argue that the repo should not contain development environment specific ignores. Contributors can/should set their own local global ignores.
There was a problem hiding this comment.
I think you're definitely technically correct on that one. But with an open source repo that may have any number of contributors (including those that haven't configured their global ignore), maybe this just makes things easier?
There was a problem hiding this comment.
Yes my thinking was the same as @agibson-godaddy -- I would prefer we maintain this as is, wouldn't bother anyone.
| sudo update-alternatives --set phar /usr/bin/phar$PHP_VERSION | ||
| sudo update-alternatives --set phar.phar /usr/bin/phar.phar$PHP_VERSION | ||
| sudo update-alternatives --set phpize /usr/bin/phpize$PHP_VERSION | ||
| sudo update-alternatives --set php-config /usr/bin/php-config$PHP_VERSION |
There was a problem hiding this comment.
I think we could use https://github.com/marketplace/actions/setup-php-action to make this setup cleaner, including the pcov piece below.
There was a problem hiding this comment.
as discussed with @llessa-godaddy we don't need to refactor that now, however we added a code coverage badge which may be handy
unfulvio-godaddy
requested review from
llessa-godaddy,
rneudorf-godaddy,
agibson-godaddy and
a team
|
|
| #- name: Upload coverage results to Coveralls | ||
| # if: ${{ env.PHP_VERSION == '8.1' }} | ||
| # run: | | ||
| # vendor/bin/php-coveralls --coverage_clover="./clover.xml" --json_path="./coveralls-upload.json" -v |
There was a problem hiding this comment.
Coveralls won't work currently because it needs some envs set up - PHPUnit Code coverage and a badge output is currently working from this PR on the other hand
### Description of the Change It's advantageous that `composer.lock` is commited to the repository. This introduces more consistency when dependencies are installed by individuals for projects depending on WP_Mock. If someone wants to learn more about why this is important (if you have a better article, please post in comment!): * https://www.amitmerchant.com/why-you-should-always-commit-the-composer-lock-file/ * https://www.codementor.io/@decodeweb/importance-of-composer-lock-in-git-14v9ujxn9z * https://m.dotdev.co/why-the-composer-lock-file-matters-c8d1e1488be7 One notable update since #167 is maybe bumping PhpUnit to the `^9.5` constraint -- however the previous constraint `>=7.0` made it so that `9.5.x` could have been installed too. I've also removed `sebastian/comparator` from dev dependencies. Not sure what this is used for since I couldn't find any reference to it. Maybe it was required in older phpunit versions? As for `behat` and `php-coveralls` we might still use those later on. Closes #153 ### How to test the Change - [x] Review composer.json and composer.lock changes - [x] Unit tests pass ### Changelog Entry - `composer.lock` is commited to git ### Checklist: - [x] I agree to follow this project's [**Code of Conduct**](https://github.com/10up/.github/blob/trunk/CODE_OF_CONDUCT.md). - [ ] I have updated the documentation accordingly. _n/a_ - [ ] I have added tests to cover my change. _n/a_ - [x] All new and existing tests pass.
Description of the Change
Replace Travis with a new CI workflow using GitHub actions.
Having tests run again on each PR should give any contributor more confidence when opening a pull request.
I had to bump min php 7.3 to avoid issues with composer. We should version composer.lock after this.
Have also added an action that generates a coverage badge based on PHPUnit test coverage output. This is a static badge and not one dynamically generated with a third party service like Coveralls, CodeCov, etc. To use Coveralls as a GitHub action looks like we would need additional setup since lcov is not a coverage format that phpunit supports. Having a static badge should be good enough though, in the future we could have it automatically updated to README.md as part of a release bump.
How to test the Change
This change is ignoring Behat at the moment. Need discussion to understand if we still need it or could limit test coverage to unit tests only. Behat could still be run by appending
vendor/bin/behatright afterphpunitin the corresponding action step.Suggested list of follow up improvements:
Changelog Entry
@coverstags to tests (now a requirement)