Migrate all schemas in AbstractSchemaDefinitions #8726
Merged
Conversation
This file contains 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
rolfyone
reviewed
Oct 15, 2024
| @@ -33,6 +35,13 @@ public class SchemaTypes { | |||
| // PHASE0 | |||
| public static final SchemaId<SszBitvectorSchema<SszBitvector>> ATTNETS_ENR_FIELD_SCHEMA = | |||
| create("ATTNETS_ENR_FIELD_SCHEMA"); | |||
| public static final SchemaId<SszBitvectorSchema<SszBitvector>> SYNCNETS_ENR_FIELD_SCHEMA = | |||
| create("SYNCNETS_ENR_FIELD_SCHEMA"); | |||
There was a problem hiding this comment.
are we going to reference by strings elsewhere? Wonder if we should use const's or an enum or something so that we're not typing the string a bunch if we are
There was a problem hiding this comment.
Ths string are not referenced elsewhere, they are just used internally (they are exposed in error logs to be able to identify where the error is in the schema definitions)
It is not possible to move to an enum (at least i haven't found any good alternative) since we need to capture the type here.
I attempted this trick to move the string as an enum but it doesn't not worth it IMO.
Btw strings are enforced to be equal to the const via unit test.
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.
To make provider type more more explicit, I added specific
constantProviderBuilderandvariableProviderBuilderrelated to #8588
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog