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.

Sign up for GitHub

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

Jump to bottom

Refactor: rename protocol parameters for consistency in signer and aggregator #2058

Merged
merged 15 commits into from
Oct 30, 2024
Merged

Refactor: rename protocol parameters for consistency in signer and aggregator #2058

merged 15 commits into from
Oct 30, 2024

Conversation

dlachaume
Copy link
Collaborator

@dlachaume dlachaume commented Oct 29, 2024 •
edited
Loading

Content

This PR includes a renaming of protocol parameters within the signer and aggregator to improve naming consistency and maintainability.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update README file (if relevant)
    • Update documentation website (if relevant)
    • Add dev blog post (if relevant)

Issue(s)

Relates to #1966

@dlachaume dlachaume self-assigned this Oct 29, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 29, 2024 •
edited
Loading

Test Results

    4 files  ±0     51 suites  ±0   10m 42s ⏱️ +15s
1 400 tests +1  1 400 ✅ +1  0 💤 ±0  0 ❌ ±0 
1 611 runs  +1  1 611 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 199d236. ± Comparison against base commit 8ee076c.

♻️ This comment has been updated with latest results.

@dlachaume
refactor(signer): rename next_protocol_parameters to `signer_regist…
3a8e9ae 
…ration_protocol_parameters` in Epoch service
@dlachaume
refactor(signer): remove unused protocol_parameters field in `Signe…
8c8ed87 
…rEpochSettings`
@dlachaume
refactor(signer): rename next_protocol_parameters in `signer_regist…
43fbaca 
…ration_protocol_parameters` in `SignerEpochSettings`
@dlachaume
refactor: deprecate protocol_parameters in EpochSettingsMessage
cee6b6d
@dlachaume
refactor: deprecate next_protocol_parameters in EpochSettingsMessage
35a7f17
@dlachaume
refactor: add signer_registration_protocol_parameters in `EpochSett…
7deee47 
…ingsMessage`
@dlachaume
refactor: adjustments in the epoch_settings message module
42d4c6b 
- remove public visibility for `EpochSettingsMessageUntilV0_1_32`
- remove unnecessary comments
- update deprecated version reference of mithril-common
@dlachaume dlachaume force-pushed the dlachaume/1966/refactor-protocol-parameters-naming branch from 8f42843 to 42d4c6b Compare October 29, 2024 11:18
@dlachaume dlachaume temporarily deployed to testing-sanchonet October 29, 2024 13:51 — with GitHub Actions Inactive
@dlachaume dlachaume marked this pull request as ready for review October 29, 2024 14:01
jpraynaud
jpraynaud approved these changes Oct 29, 2024
View reviewed changes
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 👍

mithril-aggregator/src/services/epoch_service.rs Outdated Show resolved Hide resolved
mithril-aggregator/src/services/epoch_service.rs Outdated Show resolved Hide resolved
mithril-signer/src/message_adapters/from_epoch_settings.rs Outdated Show resolved Hide resolved
sfauvel
sfauvel approved these changes Oct 29, 2024
View reviewed changes
Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

LGTM

@dlachaume dlachaume force-pushed the dlachaume/1966/refactor-protocol-parameters-naming branch from d5aee59 to b05e935 Compare October 29, 2024 16:29
@dlachaume dlachaume temporarily deployed to testing-sanchonet October 29, 2024 16:37 — with GitHub Actions Inactive
Alenar
Alenar approved these changes Oct 29, 2024
View reviewed changes
Copy link
Collaborator

@Alenar Alenar left a comment

Choose a reason for hiding this comment

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

LGTM

mithril-signer/src/entities/signer_epoch_settings.rs Outdated Show resolved Hide resolved
@dlachaume
refactor(signer): remove signer_ prefix as is implicit in the conte…
f775ec4 
…xt of the node
@dlachaume
chore: update CHANGELOG to inform deprecation of protocol and `next…
43e90bd 
…_protocol`
@dlachaume
chore: upgrade crate versions and openapi.yaml version
199d236 
* mithril-aggregator from `0.5.93` to `0.5.94`
* mithril-common from `0.4.76` to `0.4.77`
* mithril-signer from `0.2.205` to `0.2.206`
* mithril-aggregator-fake from `0.3.11` to `0.3.12`
* openapi.yaml from `0.1.33` to `0.1.34`
@dlachaume dlachaume force-pushed the dlachaume/1966/refactor-protocol-parameters-naming branch from b05e935 to 199d236 Compare October 29, 2024 17:05
@dlachaume dlachaume temporarily deployed to testing-sanchonet October 29, 2024 17:14 — with GitHub Actions Inactive
@dlachaume dlachaume merged commit 1bdd873 into main Oct 30, 2024
47 of 48 checks passed
@dlachaume dlachaume deleted the dlachaume/1966/refactor-protocol-parameters-naming branch October 30, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpraynaud jpraynaud jpraynaud approved these changes

@sfauvel sfauvel sfauvel approved these changes

@Alenar Alenar Alenar approved these changes

Assignees

@dlachaume dlachaume

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dlachaume @Alenar @sfauvel @jpraynaud