-
Notifications
You must be signed in to change notification settings - Fork 12
Improve MethodSignature
and WithTime
#1319
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
Merged
Merged
Conversation
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
Codecov Report
@@ Coverage Diff @@
## master #1319 +/- ##
============================================
- Coverage 91.06% 90.98% -0.08%
- Complexity 4748 4765 +17
============================================
Files 609 612 +3
Lines 15101 15130 +29
Branches 854 856 +2
============================================
+ Hits 13751 13766 +15
- Misses 1080 1097 +17
+ Partials 270 267 -3 |
MethodSignature
and WithTyme
MethodSignature
and WithTyme
MethodSignature
and WithTime
armiol
requested changes
Nov 17, 2020
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.
@alexander-yevsyukov LGTM in general. I really liked the shortcut types for the numerous Immutable...
collections.
Please see my comments though.
model/model-verifier/src/test/java/io/spine/model/verify/ModelVerifierPluginTest.java
Show resolved
Hide resolved
server/src/main/java/io/spine/server/entity/storage/InterfaceBasedColumn.java
Outdated
Show resolved
Hide resolved
server/src/main/java/io/spine/server/event/model/EventAcceptingMethodParams.java
Show resolved
Hide resolved
server/src/main/java/io/spine/server/event/model/ReactingClass.java
Outdated
Show resolved
Hide resolved
server/src/main/java/io/spine/server/event/model/SubscriberSignature.java
Outdated
Show resolved
Hide resolved
armiol
approved these changes
Nov 17, 2020
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR:
Adds
localDate()
andlocalDateTime()
methods to theWithTime
interface... thus saving on efforts required for obtaining popular temporals from events or commands.
Previously, only
instant()
was exposed which required callingLocalDate.from(TemporalAccessor)
orLocalDateTime.from(TemporalAccessor)
. This in turn, caused Error Prone warning because not allTemporalAccessor
s provide date information (causingDateTimeException
).New methods address removes the whole hassle because
Instant
does have both date and time information, so the exception would never occur.Bumps
base
andconfig
Refactors
MatchCriterion
I encountered Java compiler error telling “unable to instantiate” for declaration of
MatchCriterion.PROHIBITED_EXCEPTION
member. So, I extracted common bits and non-trivial message composition required forPROHIBITED_EXCEPTION
, and the compiler swallowed it.Makes
SignatureMismatch.create()
returnOptional<>
... as this is the only case we use.
Makes
MethodSignature
return only oneAccessModifier
We use this only for recommending (via warning) correct modifiers during compile time.
It would be strange to recommend two or more things because it would cause a question, “Which one to use when?”
The previous signature was:
Now it's:
Encapsulate allowed types of a
MethodSignature
Before:
After:
Encapsulates return types of method signatures
Previously,
MethodSignature.returnTypes()
returnedImmutableSet<TypeToken<?>>
.Now it's just
ReturnTypes
, and the class encapsulates the checking against aMethod
.