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.
Support reading configurations from files #8338
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
Support reading configurations from files #8338
Changes from all commits
c512b78
dc449e0
6d72f81
92c81fd
635ba8a
d5bf963
9f03e04
06d572d
a03562c
ac7b387
908a72d
abfc789
38355fe
aebebbe
90b52e9
cdd8df8
1fed1da
9f2b252
2da99f2
ae6036b
e3d6c80
a7bf5ba
0c8ef6a
049da8b
39fa0e2
53dec92
96881d0
09e22a7
519c443
9a519e1
a9f968f
7c93f15
36a7e8a
9598e11
bf9efad
a34a546
a3be88d
99cb07b
cf99c4f
3d1a1d8
41fd3bc
9dd51fe
f143632
ea8d9f7
4366cab
eb7618b
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.
Is there only one configId per file? If so then we can store this in a
String[1]
array above and only call this matcher if we haven't yet found the configIdThere 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.
Is there expected to only be one config_id per file? — Theoretically yes, but neither the spec nor the tests specify how to handle multiple the scenario where multiple config_ids are present.
With my implementation, the last config_id processed will be used. With your approach, the first one will be used.
I'm reaching out to the channel now to see if we want to standardize on this behavior.
Update: Consensus seems to be: throw an error if >1 config_id is found.
https://dd.slack.com/archives/C0838DD5SSJ/p1740074416283709
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.
It's a shame that the Java 8 stream API doesn't have the
takeWhile
anddropWhile
methods that were added in Java 9 (https://docs.oracle.com/javase/9/docs/api/java/util/stream/Stream.html#takeWhile-java.util.function.Predicate-) because that would have really helped here :)It's possible to mimic those with a custom
Spliterator
as shown in https://www.baeldung.com/java-break-stream-foreach#custom-spliteratorbut TBH I think the approach taken here is fine given the limited API available to us in Java 8