Merged
Conversation
Currently, any value can be provided at compile time, including incompatible ones. A compatibility check is then executed at runtime on line 111. By using generics on the constructor, we ask the compiler to do this check for us. Consequently, the check is done immediately (compile time) and the runtime check of line 111 becomes irrelevant. For example, in the previous commit, while replacing the END_OF_LINE_CHARACTER value from SYTEM_EOL (String) to LineSeparator.SYSTEM (LineSeparator), the type incompatibility was only spotted upon running the tests (which massively exploded), making me fix it by using LineSeparator.SYSTEM.asRawString() (String). Now we know the discrepancy immediately from the compiler, before to run any test.
matthieu-vergne
force-pushed
the
fix_EOL_warnings
branch
from
5fec589 to
e886297
Compare
Collaborator
|
That seems fine to me. Thanks for this PR. |
jlerbsc
added
the
PR: Changed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The first commit fixes the warnings themselves.
The next commit replaces a runtime check by a compile time check to solve an issue discovered while fixing the warnings (see commit description for details).
The last two commits remove the SYSTEM_EOL and EOL deprecated constants, since they are not used anymore.
If the constants should remain there for retrocompatibility, you can ignore the two last commits. I would however suggest to explicitly mention in their Javadoc that they should be removed in the future (preferably by telling the target version for removal, like the next major version or any precise reference).