Skip to content

Update Secure Cadence #516

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
Merged
turbolent merged 11 commits into from
Jun 9, 2022
Merged

Update Secure Cadence #516

turbolent merged 11 commits into from
Jun 9, 2022

Conversation

@devbugging
Copy link
Contributor

@devbugging devbugging commented Jun 8, 2022 •
edited by SupunS
Loading

This PR updates to

For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@devbugging devbugging self-assigned this Jun 8, 2022
@devbugging devbugging requested a review from turbolent June 8, 2022 09:07
@turbolent turbolent marked this pull request as ready for review June 8, 2022 23:14
@turbolent turbolent added the Improvement Technical work without new features, refactoring, improving tests label Jun 8, 2022
@turbolent turbolent changed the title Update to secure Cadence Update Secure Cadence Jun 8, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jun 8, 2022 •
edited
Loading

Codecov Report

Merging #516 (7c99b5e) into master (9b004d0) will not change coverage.
The diff coverage is 85.71%.

@@           Coverage Diff           @@
##           master     #516   +/-   ##
=======================================
  Coverage   55.05%   55.05%           
=======================================
  Files          37       37           
  Lines        2007     2007           
=======================================
  Hits         1105     1105           
  Misses        758      758           
  Partials      144      144
Flag Coverage Δ
unittests 55.05% <85.71%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/flowkit/keys.go 32.60% <0.00%> (ø)
pkg/flowkit/services/blocks.go 69.23% <100.00%> (ø)
pkg/flowkit/services/events.go 100.00% <100.00%> (ø)

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 9b004d0...7c99b5e. Read the comment docs.

devbugging
devbugging commented Jun 9, 2022
View reviewed changes
pkg/flowkit/config/config_test.go
assert.Equal(t, contracts[2].Name, "Kibble")
assert.Equal(t, contracts[3].Name, "KittyItems")
assert.Equal(t, contracts[4].Name, "KittyItemsMarket")
assert.Len(t, contracts, 5)
Copy link
Contributor Author

@devbugging devbugging Jun 9, 2022

Choose a reason for hiding this comment

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

ohh I had this on my mind so long, thank you!!! coming from JS world this order always confuses me

Copy link
Member

@turbolent turbolent Jun 9, 2022

Choose a reason for hiding this comment

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

Yeah, when we first adopted testify in Cadence, I was confused by the argument order, but got used to it. Good thing is now failures are not confusing anymore.

The require/assert.Len seems odd though, because it's inconsistent with Equal's expected/actual order.

@turbolent turbolent merged commit dffa320 into master Jun 9, 2022
@turbolent turbolent deleted the improvement/secure-cadence branch June 9, 2022 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@turbolent turbolent turbolent approved these changes

@janezpodhostnik janezpodhostnik janezpodhostnik approved these changes

@psiemens psiemens Awaiting requested review from psiemens

@Kay-Zee Kay-Zee Awaiting requested review from Kay-Zee Kay-Zee is a code owner

Assignees

@devbugging devbugging

Labels

Improvement Technical work without new features, refactoring, improving tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@devbugging @codecov-commenter @turbolent @janezpodhostnik