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
feat: Add strict mode to parser #74
feat: Add strict mode to parser #74
Changes from 19 commits
697493f
9e42db0
36ef68c
67c0b03
082bec5
0909008
042480b
10df671
e44c46c
4063751
a50e940
062bdc9
28ae7b8
e6fea72
ef06099
c228484
dd4f718
26df81c
a04d11c
ca05b43
a1c3544
fa0775b
5b89108
1d412da
a16fc7b
c4271a0
b6107bb
948ca9e
a2ea48d
38575cd
99355dd
e66a8b3
a4fc92a
12fd0e6
870cffe
694c75d
9b76d55
fcb8c8d
e56246c
b4e0daf
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.
Let's default this to
true
and update tests accordingly.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.
Started with a naive approach to get an understanding of how this change affects our existing tests:
parseArgs
tostrict:true
strict:false
I'm committing early for visibility but also I will likely need assistance updating/validating these tests. At first glance of the diff I'm wondering if we can clear things up by organizing these tests in a
strict-false.js
file or groupingstrict:false
tests in each file. I'm also fine with scoping that out to another PR.cc @shadowspawn
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 the naive approach to existing tests is fine in the first instance. If test passes as is then no need to change. If it was testing a zero-config behaviour then it needs
strict: false
. There are multiple ways to group the tests, and the original test file is a bit of a jumble.(I would like to expand the end-to-end tests a bit, or at least review the coverage, but not got to that.)