monorepo-diff-buildkite-plugin
monorepo-diff-buildkite-plugin copied to clipboard
feat: `skip_path` option in `watch`
This PR adds support for specifying a skip_path
option that support either a path or a list of paths that shouldn't result in adding the trigger
step as part of the dynamically generated pipeline.
Note even in cases where a line in the diff
is skipped due to skip_path
and there are other paths in the diff
that match path
but doesn't match skip_path
, the step will still be added to the dynamically generated pipeline. i.e. for the step to be effectively ignored, the paths in skip_path
should match all of those in the given diff
.
Motivation
The motivation for this is the lack of support for negation cases in doublestar
(the package used for glob pattern matching) See https://github.com/bmatcuk/doublestar/issues/49.
I had a use case where I needed to avoid changes in certain subdirectories to trigger a step (if there were the only changes in the diff) and I tried to use the following pattern: path: "/main/{*,!(subdir1|subdir2)/**/*}"
which should be a valid glob for filtering out subdir1
and subdir2
, but didn't work.
With this, I could instead do something like:
- path: "main/"
skip_path:
- "main/subdir1"
- "main/subdir2"
and the result would be the same.
@xzyfer could you take a look at this please? π TIA!
@voluntadpear the capability proposed is valuable but a better approach would be to use a more robust globbing library with support for pattern negation i.e. https://github.com/gobwas/glob. That way we can consolidate all the matches in the path
field.
Codecov Report
Merging #88 (8e87cc1) into master (bbda0a1) will decrease coverage by
1.07%
. The diff coverage is83.33%
.
@@ Coverage Diff @@
## master #88 +/- ##
==========================================
- Coverage 78.48% 77.40% -1.08%
==========================================
Files 4 4
Lines 158 177 +19
==========================================
+ Hits 124 137 +13
- Misses 22 27 +5
- Partials 12 13 +1
Impacted Files | Coverage Ξ | |
---|---|---|
pipeline.go | 68.42% <71.42%> (-0.16%) |
:arrow_down: |
plugin.go | 88.88% <100.00%> (-4.34%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Ξ = absolute <relative> (impact)
,ΓΈ = not affected
,? = missing data
Powered by Codecov. Last update bbda0a1...8e87cc1. Read the comment docs.
@xzyfer I've taken a look at https://github.com/gobwas/glob/ and there's also a similar issue to the one of doublestar
, as reported in https://github.com/gobwas/glob/issues/47 negation only works on a character range or single character basis rather than a character group.
Also, it seems it hasn't been actively maintained for a while now.
I've tried something like "/main/{*,!(subdir1|subdir2)/**/*}"
and many variations but I couldn't find any way to negate subdir1
and subdir2
as desired for this use case here.
I saw that the turborepo team faced the same issue (https://github.com/vercel/turborepo/issues/445) with gobwas/glob
and added a --exclude
flag to specify paths to exclude, similar to what I'm proposing in this PR (btw, excludes
sounds like an excellent alternative to skip_path
), so this seems like a valid alternative.
What do you think?
@voluntadpear Sorry for delay. how about we do something as below.
Option 1: add a new property we do something like
- path:
- "main/"
- "!main/subdir1"
- "!main/subdir2"
Basically allow the path to be either string
value of array
.
Option 2: add a new property like you suggested but rename it to exclude
.
I prefer option1 though
Just to offer some encouragement - this would be really useful to have! π