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

Rename Topic and NnsFunction values #695

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Rename Topic and NnsFunction values #695

merged 1 commit into from
Aug 12, 2024

Conversation

dskloetd
Copy link
Collaborator

Motivation

Some topics and proposal types were renamed in backend enums to better reflect reality.
To avoid confusion, we make the names here consistent with what is used in the backend code.

Changes

  1. Copied the values for the Topic and NnsFunction enums from https://github.com/dfinity/ic/blob/master/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto
  2. Removed the prefixes and converted to camelcase.
  3. Fix a typo on the reference to the original proto file.

Tests

Prepared changes in nns-dapp to make the tests there pass.

Todos

  • Add entry to changelog (if necessary).

Copy link
Contributor

size-limit report 📦

Path Size
@dfinity/ckbtc 7.91 KB (0%)
@dfinity/cketh 3.45 KB (0%)
@dfinity/cmc 1.29 KB (0%)
@dfinity/ledger-icrc 3.89 KB (0%)
@dfinity/ledger-icp 15.23 KB (0%)
@dfinity/nns 36.37 KB (+0.13% 🔺)
@dfinity/nns-proto 140.98 KB (0%)
@dfinity/sns 15.9 KB (0%)
@dfinity/utils 4.47 KB (0%)
@dfinity/ic-management 2.87 KB (0%)

@dskloetd dskloetd marked this pull request as ready for review August 12, 2024 13:18
@dskloetd dskloetd requested review from a team as code owners August 12, 2024 13:18
@dskloetd dskloetd requested a review from aterga August 12, 2024 13:19
@dskloetd dskloetd merged commit b015643 into main Aug 12, 2024
11 checks passed
@dskloetd dskloetd deleted the kloet/topic-rename branch August 12, 2024 14:06
github-merge-queue bot pushed a commit to dfinity/nns-dapp that referenced this pull request Aug 12, 2024
# Motivation

Some Topic and NnsFunction enum values in ic-js were renamed in
dfinity/ic-js#695
This is a breaking change requiring changes in nns-dapp.

# Changes

1. Ran `npm run upgrade:next`
2. Changed enum values to make tests pass.

# Tests

CI passes

# Todos

- [ ] Add entry to changelog (if necessary).
not necessary
github-merge-queue bot pushed a commit to dfinity/nns-dapp that referenced this pull request Aug 21, 2024
# Motivation

Some `nnsFunction` enum keys have been updated in
[ic-js](dfinity/ic-js#695), but the
corresponding keys in the NNS-dapp governance labels were not updated.
This discrepancy leads to missing type field for the renamed types in
the NNS-dapp. In this PR, we synchronize the `nnsFunction` label keys to
match the `ic-js` enum.

# Changes

- Rename `UpdateSubnetReplicaVersion` to
`DeployGuestosToAllSubnetNodes`.
- Rename `UpdateElectedReplicaVersions` to
`ReviseElectedGuestosVersions`.
- Rename `AddApiBoundaryNode` to `AddApiBoundaryNodes`.
- Drive-by: update `getNnsFunctionKey` test name.

# Tests

- Added a test to verify that there are labels for all provided
`nnsFunction` values.
- Tested locally to confirm that the `DeployGuestosToAllSubnetNodes`
proposal now correctly displays the type field after the changes (which
was previously missing).

# Todos

- [ ] Add entry to changelog (if necessary).
Not necessary.
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.

2 participants