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

Fix remaining unchecked arithmentic in examples #1930

Merged
merged 3 commits into from
Oct 4, 2023

Conversation

ascjones
Copy link
Collaborator

@ascjones ascjones commented Oct 3, 2023

Follow up to #1831, using checked arithmetic

@codecov-commenter
Copy link

Codecov Report

Merging #1930 (699f30f) into master (fb7e3fa) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1930      +/-   ##
==========================================
+ Coverage   52.88%   52.91%   +0.02%     
==========================================
  Files         219      219              
  Lines        6781     6781              
==========================================
+ Hits         3586     3588       +2     
+ Misses       3195     3193       -2     
Files Coverage Δ
...ests/ui/contract/pass/example-incrementer-works.rs 72.22% <100.00%> (ø)
...i/contract/pass/example-trait-incrementer-works.rs 57.14% <100.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@pgherveou pgherveou left a comment

Choose a reason for hiding this comment

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

why not saturating_add instead checked_add + unwrap?

@ascjones
Copy link
Collaborator Author

ascjones commented Oct 4, 2023

why not saturating_add instead checked_add + unwrap?

In the unlikely event of an overflow we definitely want to panic instead of wrapping around to 0

@ascjones ascjones merged commit 9a5c212 into master Oct 4, 2023
22 checks passed
@ascjones ascjones deleted the aj/examples-checked-arithmentic branch October 4, 2023 08:54
ascjones added a commit that referenced this pull request Oct 4, 2023
* Fix unchecked arithmetic on remaining integration-tests

* Fix

* Fix
ascjones added a commit that referenced this pull request Oct 17, 2023
* Update contract_size.sh script

* Add --quiet option to for_all_contracts_exec.sh

* Bash lowercase var names

* Print contract dir instead of contract name because it is unique

* TEMPORARILY comment out other steps

* Add measurements stage with contract sizes for current branch

* print result file

* Add back stages

* See if it works without the RUSTFLAGS

* Add back RUSTFLAGS

* Don't swallow build error in contract size

* Just run measurements for now...

* Ok restore the rest then

* Do measurements before examples

* Try running contract sizes for `master`

* Use upstream dir

* --quiet

* cd into upstream dir for matching names

* CI project dir

* scripts dir and add todo

* popd

* remove todo

* Add script to produce contract sizes table

* awk vars

* See if it works in a subdir

* See if it works in a subdir, use script

* before/after script

* Comment

* Try up a directory

* Fix remaining unchecked arithmentic in examples (#1930)

* Fix unchecked arithmetic on remaining integration-tests

* Fix

* Fix

* MEASUREMENTS_DIR abs

* fix script name

* rules for only running on PRs

* Generate markdown table from diff

* Submit contract markdown table

* chmod +x

* Print PR_COMMENTS_URL

* Use CI_COMMIT_REF_NAME var

* Export vars

* Don't need to cd into ink dir

* Print cargo-contract version

* Fix cargo-contract version regex

* Fix

* Try to fix json body generation

* Fix formatting

* master_ahead

* Update link

* Trim contract names with integration-tests prefix

* Add change column

* Don't display vars

* Try remove RUSTFLAGS
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.

None yet

6 participants