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

Remove pallet::getter usage from pallet-contracts-mock-network #4417

Merged

Conversation

PolkadotDom
Copy link
Contributor

@PolkadotDom PolkadotDom commented May 8, 2024

A part of #3326

Removes all #[pallet::getter] usage from the contracts mock network pallet. As the storage values were pub(super), read-only visibility was lost external to the crate upon the removal of the macros. I have implemented custom getters as a replacement, keeping the api the same.

If we care very much about consistency of the storagevalue::<T>::get() syntax, the other option would be to set the storage values to pub. Though I find preserving data authority better myself.

@muraca

@PolkadotDom PolkadotDom requested a review from athei as a code owner May 8, 2024 15:41
@bkchr bkchr added the R0-silent Changes should not be mentioned in any release notes label May 9, 2024
@bkchr
Copy link
Member

bkchr commented May 9, 2024

From the naming this is only a mock, this means internal code and we don't need a PRDOC. However, best to wait for @athei / @pgherveou to comment.

@athei athei enabled auto-merge May 10, 2024 10:00
@athei athei disabled auto-merge May 10, 2024 10:00
@athei
Copy link
Member

athei commented May 10, 2024

Yes we don't need the PRDoc but it also doesn't hurt? The network is used to run tests which need more than one node.

@bkchr bkchr added this pull request to the merge queue May 10, 2024
@bkchr
Copy link
Member

bkchr commented May 10, 2024

The network is used to run tests which need more than one node.

Not really sure what you mean by this.

Merged via the queue into paritytech:master with commit 84d6437 May 10, 2024
151 of 152 checks passed
ordian added a commit that referenced this pull request May 14, 2024
* master:
  improve MockValidationDataInherentDataProvider to support async backing (#4442)
  Bump `proc-macro-crate` to the latest version (#4409)
  [ci] Run check-runtime-migration in GHA (#4441)
  prospective-parachains rework (#4035)
  [ci] Add forklift to GHA ARC (#4372)
  `CheckWeight` SE: Check for extrinsic length + proof size combined (#4326)
  Add generate and verify logic for `AncestryProof` (#4430)
  Rococo AH: undeploy trie migration (#4414)
  Remove `substrate-frame-cli` (#4403)
  migrations: `take()`should consume read and write operation weight (#4302)
  `remote-externalities`: store block header in snapshot (#4349)
  xcm-emlator: Use `BlockNumberFor` instead of `parachains_common::BlockNumber=u32` (#4434)
  Remove `pallet::getter` usage from authority-discovery pallet (#4091)
  Remove pallet::getter usage from pallet-contracts-mock-network (#4417)
  Add docs to request_core_count (#4423)
@PolkadotDom PolkadotDom deleted the dom/remove-getters-contracts-mock-network branch May 14, 2024 21:13
@athei
Copy link
Member

athei commented May 15, 2024

We are using it for testing XCM.

hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
…ytech#4417)

A part of paritytech#3326 

Removes all #[pallet::getter] usage from the contracts mock network
pallet. As the storage values were pub(super), read-only visibility was
lost external to the crate upon the removal of the macros. I have
implemented custom getters as a replacement, keeping the api the same.

If we care very much about consistency of the
storagevalue::&lt;T&gt;::get() syntax, the other option would be to set
the storage values to pub. Though I find preserving data authority
better myself.

@muraca
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…ytech#4417)

A part of paritytech#3326 

Removes all #[pallet::getter] usage from the contracts mock network
pallet. As the storage values were pub(super), read-only visibility was
lost external to the crate upon the removal of the macros. I have
implemented custom getters as a replacement, keeping the api the same.

If we care very much about consistency of the
storagevalue::&lt;T&gt;::get() syntax, the other option would be to set
the storage values to pub. Though I find preserving data authority
better myself.

@muraca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants