Skip to content

Incremental check for coding standards in pull requests #2936

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 22, 2017
Merged

Incremental check for coding standards in pull requests #2936

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f86c8c7
Incremental check for coding standards in pull requests
morozov Dec 8, 2017
cce96b0
Unshallow the merge branch if the merge base is unreachable
morozov Dec 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,15 @@ jobs:
- wget https://scrutinizer-ci.com/ocular.phar
- php ocular.phar code-coverage:upload --format=php-clover clover.xml

- stage: Pull request coding standard
if: type = pull_request
php: 7.2
script:
- wget https://github.com/diff-sniffer/git/releases/download/0.1.0/git-phpcs.phar
- php git-phpcs.phar origin/$TRAVIS_BRANCH...$TRAVIS_PULL_REQUEST_SHA
Copy link
Member

Choose a reason for hiding this comment

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

Careful here: travis clone operations are generally shallow, which means that this may fail after 100 commits or even less.

Copy link
Member Author

@morozov morozov Dec 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

From the documentation:

Rather than test the commits that have been pushed to the branch the pull request is from, we test the merge between the origin and the upstream branch.

I was under the impression that both origin/$TRAVIS_BRANCH and $TRAVIS_PULL_REQUEST_SHA will be immediate parents of the merge commit. So they both should be on the second level of depth.

Not sure how shallow clone handles multiple parents though.

Copy link
Member Author

Choose a reason for hiding this comment

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

In theory, there indeed may be a problem when the merge base of the two parents is more than 50 commits behind any of them. So even if we fetch the merge base manually, there's no guarantee all commits in between are available. Hm…


- stage: Coding standard
if: NOT type = pull_request
php: nightly
script:
- ./vendor/bin/phpcs