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

bugfix: Ignore paths filtering with file outside of paths #14

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

denniskpw
Copy link

When paths is defined and ignore_paths is has the coverage of one more subdirectory of paths, there can be a scenario where a pull request contains 2 files. The first file exists within ignore_paths and the second file does not match paths nor ignore_paths. The expectation is no new resource version should be created, but since we match on the number of files in the pull request with the number of files matched by the ignore path (in our example 2 and 1 respectively), because 2 != 1, the earlier logic to ignore the pull request would not have been satisfied (hence continue would not have been executed). Instead, we must also check that there are no files that exist after applying both paths and ignore_paths filter.

… the other does not

When paths and ignore_paths are both defined, and the pull request
contains one file in the ignored path and another outside of paths
and outside ignore_paths, a new resource version is created because
the number of files matching ignore_paths does not match the number
of files changed. This behaviour is incorrect because there may be
changed files that do not match paths and do not match ignore_paths.
@denniskpw denniskpw force-pushed the bugfix/paths_ignore_paths_filtering branch from b16034a to cce6c13 Compare April 9, 2020 19:28
Copy link
Contributor

@christophermancini christophermancini left a comment

Choose a reason for hiding this comment

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

Only reviewing this to get it off my list of review requested PRs. I am no longer involved in this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants