Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Pass args to coverage and profiler #2592
base: master
Are you sure you want to change the base?
Pass args to coverage and profiler #2592
Changes from all commits
6ea28b2
cac1d21
9001076
3366a07
8f9063c
77d9cf7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 suppose that such heuristic can generate problems in the future, and introducing such condition disturbs the readability of the code. Maybe we should just add coverage subcommand to catch arguments in simpler and cleaner way? What do you think?
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 we could at least check if
--coverage
or--build-profile
were actually passed. Because I think this heuristic should be only enabled in this case, otherwise, we shouldn't be doing anything with the 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.
We could consider some warning when it runs, but not sure what it would be,
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.
yes, it's the correct one, but it is going to be the same thing as if there was just a subcommand.
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 guess we were discussing this earlier i don't know why we bring it here now and block it with the request changes 😅
There shouldn't be any warning.
snforge test --coverage --my-coverage-flag
is perfectly fine and user doesn't do anything wrong and yet it caputers --my-coverage-flag in test filterWhat about some meaningful validation of argument passed to filter? I don't think we should allow passing test filter argument starting
-
.