-
Notifications
You must be signed in to change notification settings - Fork 260
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
Siemens CSA Reader: Increase maximum number of items #690
Conversation
I've encountered a file with 282 items. It might even make sense to do away with this limit altogether. |
This seems fine to me. A limit of 99 was added by @matthew-brett in 59dc04a and updated to 199 by @kevlarkevin in #242. Could one or both of you chime in on the original intent, and whether it makes sense to keep a fixed limit at all? @isolovey-robarts In any event, you may want to update the test added in #242. |
No principled reason for the number, just there to avoid getting locked into parsing a broken header into the rest of the filesystem. |
f870691
to
0dd290d
Compare
Kept the limit at 1000 and fixed the tests. |
Please merge master to fix style checks. |
0dd290d
to
8c90a20
Compare
Codecov Report
@@ Coverage Diff @@
## master #690 +/- ##
=======================================
Coverage 88.88% 88.88%
=======================================
Files 93 93
Lines 11449 11449
Branches 1892 1892
=======================================
Hits 10176 10176
Misses 933 933
Partials 340 340
Continue to review full report at Codecov.
|
Fails on testing against pre-release builds, but nothing to do with this pull request as far as I can tell. |
I agree. Thanks! |
No description provided.