REOPEN: [#1466] Filter intersection of tags #1968
Conversation
…ctions which include all tags in --tag option
|
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. |
|
|
||
| const hasActionSelector = runConfig.actions?.length > 0; | ||
| const hasTagSelector = runConfig.tags?.length > 0; | ||
| const hasFlterTagsAsIntersectionSelector = runConfig.filterTagsAsIntersection?.valueOf(); |
There was a problem hiding this comment.
Instead of runConfig.filterTagsAsIntersection?.valueOf() I suggest to use !!runConfig.filterTagsAsIntersection
There is also a typo in the word Filter.
| } | ||
| }; | ||
|
|
||
| const flterTagsAsIntersectionSelectorOption: INamedOption<yargs.Options> = { |
| }; | ||
|
|
||
| const flterTagsAsIntersectionSelectorOption: INamedOption<yargs.Options> = { | ||
| name: "filter-tags-as-intersection", |
There was a problem hiding this comment.
Let's make it shorter for the user. Maybe just tags-intersection, WDYT?
| test("prune actions with --tags (with include all tags)", () => { | ||
| const prunedGraph = prune(TEST_GRAPH_WITH_TAGS, { | ||
| tags: ["tag1", "tag2"], | ||
| includeAllTags: true |
| expect(actionNames).not.includes("schema.op_a"); | ||
| expect(actionNames).not.includes("schema.op_b"); | ||
| expect(actionNames).not.includes("schema.op_c"); | ||
| expect(actionNames).not.includes("schema.op_d"); |
There was a problem hiding this comment.
We already made a deep.equals on previous line, we do not need to double check this with not.include
|
@zsomborkincses do you plan to work on this? Otherwise I will close the PR. |
|
Closing this because there is no response from the PR author. |
2 participants
Reopened a feature request that has been opened by @AnOtterGithubUser and abandoned (1467) based on the issue: 1466
Included changes based on the review of @Ekrekr