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

Check config before skipping #842

Merged
merged 2 commits into from
Jan 29, 2021
Merged

Check config before skipping #842

merged 2 commits into from
Jan 29, 2021

Conversation

dhaarbrink
Copy link
Contributor

@dhaarbrink dhaarbrink commented Nov 18, 2020

This adds a check on the 'use_grumphp_paths' config before skipping so this task runs even though there are no changed files when use_grumphp_paths is set to false.

Q A
Branch master for features and deprecations
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Documented? no
Fixed tickets -

New Task Checklist:

  • Are the dependencies added to the composer.json suggestions?
  • Is the doc/tasks.md file updated?
  • Are the task parameters documented?
  • Is the task registered in the tasks.yml file?
  • Does the task contains phpunit tests?
  • Is the configuration having logical allowed types?
  • Does the task run in the correct context?
  • Is the run() method readable?
  • Is the run() method using the configuration correctly?
  • Are all CI services returning green?

@veewee
Copy link
Contributor

veewee commented Nov 26, 2020

Thanks for the PR! It makes sense to do it like this.

Can you also add a unit test covering this case?
There is already a test for when there are files available, so it should be a small addition:
https://github.com/phpro/grumphp/blob/master/test/Unit/Task/PhpStanTest.php#L192

@Landerstraeten
Copy link
Contributor

@veewee Can you check if I have added the correct unit test?

@Landerstraeten Landerstraeten merged commit b5a08f6 into phpro:master Jan 29, 2021
@Landerstraeten
Copy link
Contributor

@dhaarbrink Thank you!

@veewee veewee added this to the 1.3.1 milestone Jan 29, 2021
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