-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(filter): error on invalid filters #8142
feat(filter): error on invalid filters #8142
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Ignored Deployments
|
🟢 Turbopack Benchmark CI successful 🟢Thanks |
🟢 CI successful 🟢Thanks |
ce72c59
to
1b233da
Compare
1b233da
to
39e1c18
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, reviewed by commit. What happens if -F ./packages/*
is given and packages
dir exists, but doesn't have any subdirectories? Do we care about erroring on that case?
@@ -2,13 +2,17 @@ Setup | |||
$ . ${TESTDIR}/../../../helpers/setup_integration_test.sh | |||
|
|||
# Save JSON to tmp file so we don't need to keep re-running the build | |||
$ ${TURBO} run build --dry=json --filter=main > tmpjson.log | |||
$ ${TURBO} run build --dry=json --filter='[main]' > tmpjson.log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I wrote these tests? but I am not sure if even this is valid usage of --filter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you wanted to write a filter for changes since main, but without the brackets it was just a filter for the non-existent main
package.
I don't think we should since we decided to only error on clearly wrong filters and not all filters that don't run tasks. For a similar reason we don't error if the user runs |
### Description This PR changes `--filter` so it now will error on certain malformed filters: - Name filters with no globs that don't match any packages e.g. `--filter=fo` instead of `--filter=foo` - Directory filters that reference a directory that doesn't exist: e.g. `--filter='./pakcages/*'` Each commit of the PR can be reviewed on it's own. ### Testing Instructions Updated existing unit tests. Added additional ones along with an integration test.
### Description This PR changes `--filter` so it now will error on certain malformed filters: - Name filters with no globs that don't match any packages e.g. `--filter=fo` instead of `--filter=foo` - Directory filters that reference a directory that doesn't exist: e.g. `--filter='./pakcages/*'` Each commit of the PR can be reviewed on it's own. ### Testing Instructions Updated existing unit tests. Added additional ones along with an integration test.
### Description This PR changes `--filter` so it now will error on certain malformed filters: - Name filters with no globs that don't match any packages e.g. `--filter=fo` instead of `--filter=foo` - Directory filters that reference a directory that doesn't exist: e.g. `--filter='./pakcages/*'` Each commit of the PR can be reviewed on it's own. ### Testing Instructions Updated existing unit tests. Added additional ones along with an integration test.
### Description This PR changes `--filter` so it now will error on certain malformed filters: - Name filters with no globs that don't match any packages e.g. `--filter=fo` instead of `--filter=foo` - Directory filters that reference a directory that doesn't exist: e.g. `--filter='./pakcages/*'` Each commit of the PR can be reviewed on it's own. ### Testing Instructions Updated existing unit tests. Added additional ones along with an integration test.
### Description This PR changes `--filter` so it now will error on certain malformed filters: - Name filters with no globs that don't match any packages e.g. `--filter=fo` instead of `--filter=foo` - Directory filters that reference a directory that doesn't exist: e.g. `--filter='./pakcages/*'` Each commit of the PR can be reviewed on it's own. ### Testing Instructions Updated existing unit tests. Added additional ones along with an integration test.
### Description This PR changes `--filter` so it now will error on certain malformed filters: - Name filters with no globs that don't match any packages e.g. `--filter=fo` instead of `--filter=foo` - Directory filters that reference a directory that doesn't exist: e.g. `--filter='./pakcages/*'` Each commit of the PR can be reviewed on it's own. ### Testing Instructions Updated existing unit tests. Added additional ones along with an integration test.
@chris-olszewski turbo run build --filter='core[main...HEAD]' it works with Turbo v1, but it will throw errors with Turbo v2. |
Description
This PR changes
--filter
so it now will error on certain malformed filters:--filter=fo
instead of--filter=foo
--filter='./pakcages/*'
Each commit of the PR can be reviewed on it's own.
Testing Instructions
Updated existing unit tests. Added additional ones along with an integration test.