Skip to content
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

feat: Support for Yaml configuration and List values #17088

Merged
merged 7 commits into from
Dec 19, 2024

Conversation

timo0
Copy link
Member

@timo0 timo0 commented Dec 17, 2024

Part of #17075

@timo0 timo0 added this to the v0.58 milestone Dec 17, 2024
@timo0 timo0 self-assigned this Dec 17, 2024
@timo0 timo0 requested review from a team as code owners December 17, 2024 16:34
@timo0 timo0 requested review from mxtartaglia-sl, povolev15 and hendrikebbers and removed request for mxtartaglia-sl December 17, 2024 16:34
Signed-off-by: Timo Brandstätter <[email protected]>
Signed-off-by: Timo Brandstätter <[email protected]>
Copy link

codacy-production bot commented Dec 17, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+2.07% (target: -1.00%) 73.68%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (720d1e0) 98488 64924 65.92%
Head commit (d1cf1bc) 95639 (-2849) 65027 (+103) 67.99% (+2.07%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#17088) 209 154 73.68%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Copy link

codecov bot commented Dec 17, 2024

Codecov Report

Attention: Patch coverage is 64.59330% with 74 lines in your changes missing coverage. Please review.

Project coverage is 64.24%. Comparing base (720d1e0) to head (d1cf1bc).
Report is 14 commits behind head on develop.

Files with missing lines Patch % Lines
...ds/config/extensions/sources/YamlConfigSource.java 56.41% 28 Missing and 6 partials ⚠️
.../config/extensions/sources/MappedConfigSource.java 70.83% 8 Missing and 6 partials ⚠️
...wirlds/config/impl/internal/ConfigurationImpl.java 55.55% 4 Missing and 4 partials ⚠️
.../config/extensions/sources/SimpleConfigSource.java 84.61% 2 Missing and 2 partials ⚠️
...s/config/impl/converters/InetAddressConverter.java 20.00% 4 Missing ⚠️
...edera/node/config/sources/DynamicConfigSource.java 0.00% 2 Missing ⚠️
...wirlds/config/impl/internal/ConfigDataFactory.java 33.33% 1 Missing and 1 partial ⚠️
...va/com/hedera/node/config/VersionedConfigImpl.java 0.00% 1 Missing ⚠️
...dera/node/config/sources/PropertyConfigSource.java 50.00% 1 Missing ⚠️
...onfig/extensions/sources/AbstractConfigSource.java 50.00% 1 Missing ⚠️
... and 3 more
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop   #17088      +/-   ##
=============================================
- Coverage      64.25%   64.24%   -0.01%     
- Complexity     20824    20857      +33     
=============================================
  Files           2547     2549       +2     
  Lines          95723    95876     +153     
  Branches       10018    10033      +15     
=============================================
+ Hits           61503    61592      +89     
- Misses         30608    30658      +50     
- Partials        3612     3626      +14     
Files with missing lines Coverage Δ
...dera/node/config/converter/AccountIDConverter.java 70.00% <ø> (ø)
...nfig/converter/CongestionMultipliersConverter.java 100.00% <ø> (ø)
...era/node/config/converter/ContractIDConverter.java 70.00% <ø> (ø)
.../config/converter/EntityScaleFactorsConverter.java 100.00% <ø> (ø)
.../hedera/node/config/converter/FileIDConverter.java 70.00% <ø> (ø)
...a/node/config/converter/KeyValuePairConverter.java 100.00% <ø> (ø)
...de/config/converter/KnownBlockValuesConverter.java 100.00% <ø> (ø)
...edera/node/config/converter/LongPairConverter.java 100.00% <ø> (ø)
...ra/node/config/converter/ScaleFactorConverter.java 100.00% <ø> (ø)
...va/com/swirlds/config/api/source/ConfigSource.java 0.00% <ø> (-60.00%) ⬇️
... and 32 more

... and 33 files with indirect coverage changes

Impacted file tree graph

Copy link
Member

@hendrikebbers hendrikebbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next to the given comments an update of the config documentation is needed.

Signed-off-by: Timo Brandstätter <[email protected]>
@timo0 timo0 requested a review from a team as a code owner December 18, 2024 14:38
Signed-off-by: Timo Brandstätter <[email protected]>
Copy link
Contributor

@anthony-swirldslabs anthony-swirldslabs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • platform-sdk/swirlds-config-extensions/src/main/java/module-info.java

LGTM.

Please let me know if you want me to review any other code, although I'm not an owner of anything else in this PR.

Signed-off-by: Timo Brandstätter <[email protected]>
Copy link
Member

@Neeharika-Sompalli Neeharika-Sompalli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @timo0

@timo0 timo0 merged commit 2e8fcc9 into develop Dec 19, 2024
54 of 55 checks passed
@timo0 timo0 deleted the feat-config-list-values branch December 19, 2024 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants