-
Notifications
You must be signed in to change notification settings - Fork 143
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
chore: create a state signer component #10411
chore: create a state signer component #10411
Conversation
...sdk/swirlds-config-impl/src/main/java/com/swirlds/config/impl/internal/ConverterService.java
Outdated
Show resolved
Hide resolved
...sdk/swirlds-config-impl/src/main/java/com/swirlds/config/impl/internal/ConverterService.java
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #10411 +/- ##
==========================================
Coverage ? 63.02%
Complexity ? 30802
==========================================
Files ? 3360
Lines ? 135283
Branches ? 14075
==========================================
Hits ? 85258
Misses ? 46680
Partials ? 3345 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Lazar Petrovic <lpetrovic05@gmail.com>
Signed-off-by: Lazar Petrovic <lpetrovic05@gmail.com>
Signed-off-by: Lazar Petrovic <lpetrovic05@gmail.com>
Signed-off-by: Lazar Petrovic <lpetrovic05@gmail.com>
Signed-off-by: Lazar Petrovic <lpetrovic05@gmail.com>
Signed-off-by: Lazar Petrovic <lpetrovic05@gmail.com>
Signed-off-by: Lazar Petrovic <lpetrovic05@gmail.com>
e8d6481
to
f8904cf
Compare
...ore/src/main/java/com/swirlds/platform/components/state/DefaultStateManagementComponent.java
Show resolved
Hide resolved
.../swirlds-platform-core/src/main/java/com/swirlds/platform/eventhandling/TransactionPool.java
Show resolved
Hide resolved
...-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/PlatformSchedulers.java
Show resolved
Hide resolved
if (converter == null && targetClass.isEnum()) { | ||
// FUTURE WORK: once logging is added to this module, log a warning here | ||
// ("No converter defined for type '" + targetClass + "'. Converting using backup enum converter."); | ||
try { | ||
return (T) Enum.valueOf((Class<Enum>) targetClass, value); | ||
} catch (final IllegalArgumentException e) { | ||
throw new IllegalArgumentException( | ||
"Can not convert value '%s' of Enum '%s' by default. Please add a custom config converter." | ||
.formatted(value, targetClass), | ||
e); | ||
} | ||
} |
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.
This is cool, the only problem that I see is that after this change, the method
getConverterForType(@NonNull final Class<T> valueType)
would work inconsistently compared with #convert.
Regardless I don't think that the change required to address my comment is in the scope of the current task. What do you think @hendrikebbers should we create a follow-up task to make those two behaviors consistent on our side?
closes #10319