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

Update contracts-compatibility-checker #3631

Merged
merged 4 commits into from
Oct 22, 2024
Merged

Conversation

SupunS
Copy link
Member

@SupunS SupunS commented Oct 18, 2024

Depends on onflow/flow-go#6571

Description

For each PR, the contracts backward-compatibility checker runs with the latest changes (~HEAD). The CI always replace the dependency to be the latest, however, if the tool wasn't updated to the latest cadence changes, then it is only caught at the time of CI.

Therefore, make the tool to always use the latest cadence, so any changes that would break the tool is caught locally, when running the tests.


  • 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

Copy link

github-actions bot commented Oct 18, 2024

Cadence Benchstat comparison

This branch with compared with the base branch onflow:master commit 1662018
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -benchmem -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

@SupunS SupunS force-pushed the supun/update-contracts-checker branch from 18d0e6d to 100508f Compare October 18, 2024 21:56
@SupunS SupunS marked this pull request as ready for review October 18, 2024 21:56
@SupunS SupunS self-assigned this Oct 18, 2024
@SupunS SupunS force-pushed the supun/update-contracts-checker branch from 100508f to f52a7ed Compare October 18, 2024 22:00
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice!

@SupunS SupunS force-pushed the supun/update-contracts-checker branch from 0d1172f to cfd6412 Compare October 21, 2024 17:59
@SupunS SupunS force-pushed the supun/update-contracts-checker branch from 871780e to 8408a3f Compare October 21, 2024 21:21
@SupunS
Copy link
Member Author

SupunS commented Oct 21, 2024

CI check (https://github.com/onflow/cadence/actions/runs/11448933364/job/31853489202?pr=3631), is failing at "checking contracts using master" step because the tool's code on master is outdated, due to the changes of this PR. But the workflow passes for "checking contracts using head of the PR" (as expected), and the Crypto contract is properly resolved there. So the CI-check would be back to normal once this PR is merged.

@SupunS SupunS requested a review from turbolent October 21, 2024 21:35
@SupunS SupunS changed the title Always keep compatibility-checker up to date with current changes Update contracts-compatibility-checker Oct 21, 2024
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Great work! Thanks for fixing this up

@SupunS SupunS merged commit 8b27276 into master Oct 22, 2024
10 of 12 checks passed
@SupunS SupunS deleted the supun/update-contracts-checker branch October 22, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants