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

Re-organize the content of the *.proto files. #14755

Merged
merged 4 commits into from
Dec 30, 2024
Merged

Conversation

nalepae
Copy link
Contributor

@nalepae nalepae commented Dec 28, 2024

What type of PR is this?
Other

What does this PR do? Why is it needed?
The PR re-organizes the content of the *.proto files.

The content of the *.proto files is sorted by hard fork, then with a top-down fashion.

Sorting first by hard fork lets the reader to easily see new or modified fields. Then, sorting with a top-down fashion lets the user to first see the big picture, then to dive into details.

Also, the new in <hard fork> mentions are only written for the given hard fork. Thus, it'll avoid in the future the majority of the fields, not initially present in phase 0, to have the new in <hard fork> mention.

This commit does not bring any new functional change.

Other notes for review
If the proposed organisation does not suit you, I have no objection at all to change it.
My main concern is to have something consistent in the same file and across the files.

The diff is quite large, but is actually mostly due to the re-organization of generated files due to the re-organization of the
*.proto files.

Acknowledgements

  • I have read CONTRIBUTING.md.
  • I have made an appropriate entry to CHANGELOG.md.
  • I have added a description to this PR with sufficient context for reviewers to understand this PR.

The content of the `*.proto` files is sorted by hard fork,
then with a top-down fashion.

Sorting first by hard fork lets the reader to easily see new or modified fields.
Then, sorting with a top-down fashion lets the user to first see the big picture,
then to dive into details.

Also, the `new in <hard fork>` mentions are only written for the given hard fork.
Thus, it'll avoid in the future the majority of the fields, not initially
present in phase 0, to have the `new in <hard fork> mention`.

This commit does not bring any new functional change.
@nalepae nalepae force-pushed the re-org-proto-content branch from c89ac31 to 4c7b4e0 Compare December 28, 2024 18:31
@nalepae nalepae added the Cleanup Code health! label Dec 28, 2024
@nalepae nalepae marked this pull request as ready for review December 28, 2024 18:56
@nalepae nalepae requested a review from a team as a code owner December 28, 2024 18:56
@nalepae nalepae changed the title Re-organize thet content of the *.proto files. Re-organize the content of the *.proto files. Dec 28, 2024
terencechain
terencechain previously approved these changes Dec 29, 2024
proto/prysm/v1alpha1/attestation.proto Outdated Show resolved Hide resolved

// 96 byte BLS aggregate signature signed by the aggregator over the message.
bytes signature = 2 [(ethereum.eth.ext.ssz_size) = "96"];
message SingleAttestation {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an Electra type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 44b89cd.

proto/prysm/v1alpha1/beacon_state.proto Outdated Show resolved Hide resolved
@nalepae nalepae enabled auto-merge December 30, 2024 11:16
@nalepae nalepae added this pull request to the merge queue Dec 30, 2024
Merged via the queue into develop with commit c7b2838 Dec 30, 2024
15 checks passed
@nalepae nalepae deleted the re-org-proto-content branch December 30, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Code health!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants