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

Name API consistently: transferred_balancetransferred_value #1063

Merged
merged 2 commits into from
Dec 2, 2021

Conversation

cmichi
Copy link
Collaborator

@cmichi cmichi commented Dec 1, 2021

I honestly don't care what we rename it to, but we should finally make the naming consistent.

I'm not opening up the whole thing with renaming endowment to value that was started in paritytech/substrate#10082 here.

@paritytech-ci
Copy link

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

These are the results when building the examples/* contracts from this branch with cargo-contract 0.16.0-78ea802 and comparing them to ink! master:

Δ Optimized Size Δ Used Gas Total Optimized Size Total Used Gas
accumulator 1.74 K
adder 2.87 K
contract-terminate 1.50 K 200_571
contract-transfer 8.02 K 727
delegator 9.95 K 6_151
dns 21.69 K 18_864
erc1155 47.30 K 54_061
erc20 10.34 K 6_414
erc721 36.21 K 66_677
flipper 2.02 K 173
incrementer 1.94 K 166
multisig 46.25 K 63_165
proxy 3.90 K 2_112
rand-extension 5.14 K 5_553
subber 2.89 K
trait-erc20 27.50 K 15_834
trait-flipper 1.83 K 168
trait-incrementer 1.93 K 332

Link to the run | Last update: Wed Dec 1 20:34:38 CET 2021

@codecov-commenter
Copy link

Codecov Report

Merging #1063 (06b678c) into master (44c51d1) will decrease coverage by 3.38%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1063      +/-   ##
==========================================
- Coverage   78.74%   75.35%   -3.39%     
==========================================
  Files         248      248              
  Lines        9371     9369       -2     
==========================================
- Hits         7379     7060     -319     
- Misses       1992     2309     +317     
Impacted Files Coverage Δ
crates/env/src/api.rs 36.36% <0.00%> (ø)
crates/env/src/backend.rs 80.64% <ø> (ø)
...tes/env/src/engine/experimental_off_chain/impls.rs 54.12% <0.00%> (ø)
crates/env/src/engine/off_chain/impls.rs 42.37% <0.00%> (ø)
...ng/codegen/src/generator/trait_def/call_builder.rs 100.00% <ø> (ø)
crates/lang/macro/src/lib.rs 100.00% <ø> (ø)
crates/lang/src/codegen/dispatch/execution.rs 0.00% <0.00%> (ø)
crates/lang/src/env_access.rs 12.00% <0.00%> (ø)
crates/lang/tests/ui/contract/pass/env-access.rs 3.84% <0.00%> (ø)
crates/lang/ir/src/ir/contract.rs 0.00% <0.00%> (-100.00%) ⬇️
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44c51d1...06b678c. Read the comment docs.

@ascjones
Copy link
Collaborator

ascjones commented Dec 2, 2021

but we should finally make the naming consistent

☝️ 💯

Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -357,7 +357,7 @@ impl TypedEnvBackend for EnvInstance {
self.get_property_inplace::<T::AccountId>(ext::caller)
}

fn transferred_balance<T: Environment>(&mut self) -> T::Balance {
fn transferred_value<T: Environment>(&mut self) -> T::Balance {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we really wanted to nitpick on the consistency then the seal fn value_transferred should also be the same 😀

@cmichi cmichi merged commit 378d83f into master Dec 2, 2021
@cmichi cmichi deleted the cmichi-rename-transferred-balance branch December 2, 2021 10:03
xgreenx pushed a commit to Supercolony-net/ink that referenced this pull request Feb 8, 2022
…se-ink#1063)

* Name API consistently: `transferred_balance` ➔ `transferred_value`

* Apply `cargo fmt`
freespirit added a commit to freespirit/ink-docs that referenced this pull request Jul 22, 2023
transferred_balance was renamed to transferred_value in use-ink/ink#1063
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.

4 participants