-
Notifications
You must be signed in to change notification settings - Fork 138
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
Adding staged contracts migration output JSON from Apr 17 to migratio… #3273
Conversation
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:master commit 922e65c Collapsed results for better readability
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
One quick question: at the end of the filename, what does "103000" represent so that I can parse/interpret it correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit: Maybe improve the naming an make it explicit this is the report for the staged contracts, not the staged contracts themselves. We might add them in the future, so maybe avoid the potential confusion.
@jribbink I changed the format slightly - I added the timestamp (10-30) after the date. @turbolent I also added "report" to make it clearer, good suggestion! |
migrations_data/staged-contracts-report--04-17-2024-10-30-testnet.json
Outdated
Show resolved
Hide resolved
migrations_data/staged-contracts-report--04-17-2024-10-30-testnet.json
Outdated
Show resolved
Hide resolved
@jribbink Please go ahead with making improvements as you deem appropriate, e.g. defining the naming scheme and renaming the existing report and the new one here, so you can continue on onflow/flow-cli#1529 🙏 |
@jribbink The colons in the filenames are causing issues in CI, see https://github.com/onflow/cadence/actions/runs/8820926221/job/24215623175#step:10:10. Could you please remove/replace them? |
I've made a PR to fix this 👍 #3281 |
related to #3265