REOPEN: [#1466] Filter intersection of tags#1968
REOPEN: [#1466] Filter intersection of tags#1968zsomborkincses wants to merge 1 commit intodataform-co:mainfrom
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. |
Ceridan
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
|
|
||
| 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. |
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