feat: allow excluding file paths to filter out commits #1875
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
product-auto-label
bot
added
size: l
alex-enchi
force-pushed
the
exclude-commits
branch
from
25e80a5
to
1743066
Compare
alex-enchi
changed the title
src/util/commit-exclude.ts
Outdated
| !commit.files.every(file => | ||
| excludePaths.some(path => file.indexOf(`${path}/`) === 0) | ||
| ) |
There was a problem hiding this comment.
I think there's an edge case here that will be difficult to account for if you split the exclude from the include logic. The original CommitSplit class does not strip files out that don't match the path -- it's just used to determine if that commit touches that directory (it can touch multiple components).
This looks like it's checking if there's any file that does not match the exclude path pattern. Consider a commit that touches:
a/b/cd/e/fd/e/g
We have 2 components with paths a and d and d is configured to exclude d/e/*. This commit has a file (a/b/c) that does not match the exclude pattern, and will not be excluded. If I understand the feature correctly, this commit should be excluded (there's no commit under path d that does not include d/e/*).
There was a problem hiding this comment.
hm, true there is such case.
Main reason for splitting this into two (aside of not adding additional logic tree to commit split) was the need to handle exclusion on . root scope
Thanks for catching up that usecase! Will update soon with changes
There was a problem hiding this comment.
I spent some time on adding the logic into commit split, but in the end decided to handle exclusion logic separately.
I have updated it to handle exclusion only based on relevant to project files.
Main reasoning of keeping it as "split" then "filter" is logic is a bit cleaner that way
- no special case to iterate over
. - commit split is not bloated with additional cases to handle and does only 1 thing, it splits commits per package
What do you think?
All my attempts to include that logic in commit-split itself did not look pretty, unfortunately.
There was a problem hiding this comment.
@chingor13 i think now it covers all needed usecases. Could you take a look when you have time?
Idea is to exclude commits if all files in those commits are from specific paths.
In my case I have multiple packages in
packagesand.umbrella package. Some of packages are different and handled by different workflow and I need to exclude those from umbrella.Tested only with
config-fileNote: I think it is possible to break it with
"package/foo":{"exclude-paths":["package/foo"]}but I did not test it yet.Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #908🦕