Skip to content
This repository has been archived by the owner on Jan 6, 2022. It is now read-only.

feat: add support to filtering by workspaces #4

Closed
wants to merge 2 commits into from

Conversation

ruyadorno
Copy link
Contributor

Adds support to a new workspaces option, that accepts an array of
strings, allowing users to filter the resulting fund info to a specific
set of workspaces and their dependencies.

References

Relates to: npm/statusboard#301

index.js Outdated Show resolved Hide resolved
Copy link
Contributor

@isaacs isaacs left a comment

Choose a reason for hiding this comment

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

Mutating the tree is probably going to have some unfortunate effects. Better to get a list of what you care about, and only walk those portions of it. (This is also a fair bit cheaper, since it doesn't have to update edges or walk all the nodes.)

Also, the argument should be strictly workspace names. Allowing it to be either names or paths opens the door for ambiguity. (Consider a workspace named foo at the path ./bar, and a workspace named bar at the path ./foo. What does workspaces:['foo'] mean?) We use names everywhere else, since the CLI has that handy, and you have the tree anyway.

Definitely should not land this before you can get Arborist to just hand you a set of nodes that are relied on by some list of workspace names.

test.js Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
ruyadorno added 2 commits May 13, 2021 12:06
Adds support to a new `workspaces` option, that accepts an array of
strings, allowing users to filter the resulting fund info to a specific
set of workspaces and their dependencies.

Relates to: npm/statusboard#301
@ruyadorno ruyadorno force-pushed the add-workspaces-filter branch from 3a82335 to f38ea0c Compare May 13, 2021 16:07
@isaacs isaacs self-requested a review May 13, 2021 16:08
Copy link
Contributor

@isaacs isaacs left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit:

@ruyadorno ruyadorno closed this in 46bc990 May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants