-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add code size and gas report to github CI #479
base: v2.3
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,70 @@ | |||
name: Gas Report and Code Size |
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.
One thing that's nice about these tools is that if there is a previous report it'll show the diff as well. Is there any way we could get that working? We might need to commit the reports to the repo
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.
Oh I see that the code size one does already have the diff
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.
Yeah gas diff would be incredible
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.
The tool shows the difference from the last run only. I looked but couldn't find a way to get diff from a previous report. @arjun-io
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.
Will need to play around in the gas report script to get a diff. Will try that tomorrow.
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.
Seems the goal here is to pull the "previous" report from the merge target branch. That implies reports for each branch need to be stored somewhere. Would also be nice to have this for coverage, though out-of-scope for this PR.
As a reviewer, this information should reveal how much larger the PR is making the code, and how much additional gas will be consumed by transactions impacted by the change.
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.
Right, the goal is to compare the code size changes and gas changes from base branch. Using a script I tried to compare code size and gas report tables among two branches and show the diff in a single table but that is causing a lot of issues so I think I should not spend more time on this.
I am thinking of running gas report on both the branches in the CI itself and printing two tables one below the other in a single github comment.
…d PR branch" This reverts commit 39961c1.
Code Size:
|
Gas Report:
|
[Periphery] Unit Test Coverage ReportCoverage after merging prateek/pe-1866-code-size-gas-report into v2.3 will be
Coverage Report
|
[Core] Integration Test Coverage ReportCoverage after merging prateek/pe-1866-code-size-gas-report into v2.3 will be
Coverage Report
|
[Periphery] Integration Test Coverage ReportCoverage after merging prateek/pe-1866-code-size-gas-report into v2.3 will be
Coverage Report
|
[Periphery] Combined Test Coverage ReportCoverage after merging prateek/pe-1866-code-size-gas-report into v2.3 will be
Coverage Report
|
[Core] Unit Test Coverage ReportCoverage after merging prateek/pe-1866-code-size-gas-report into v2.3 will be
Coverage Report
|
[Core] Combined Test Coverage ReportCoverage after merging prateek/pe-1866-code-size-gas-report into v2.3 will be
Coverage Report
|
No description provided.