NET-61 ci gamma diff report #1008
Conversation
jterzis
requested review from
texuf,
mechanical-turk and
brianathere
as code owners
WalkthroughThe pull request introduces a new GitHub Actions workflow, Changes
Tip New review modelWe have updated our review workflow to use the Anthropic's Claude family of models. Please share any feedback in the discussion post on our Discord. New featuresWalkthrough comment now includes:
Notes:
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Outside diff range, codebase verification and nitpick comments (2)
scripts/bytecode-diff/scripts/upgrade-facets.sh (1)
7-10: Use offind_most_recent_filefunction.The function
find_most_recent_fileis a useful utility for finding the most recent file in a directory. However, consider handling the case where the directory might be empty or the find command fails, to provide a more robust solution.scripts/bytecode-diff/cmd/root.go (1)
30-207: Comprehensive CLI setup with robust flag handlingThe CLI setup in
root.gois well-structured with clear fallbacks for environment variables and flags. Consider enhancing the help messages or documentation to provide more detailed descriptions of each flag and environment variable, which would aid users in understanding their options and configurations.
There was a problem hiding this comment.
Actionable comments posted: 7
Outside diff range, codebase verification and nitpick comments (3)
scripts/bytecode-diff/scripts/upgrade-facets.sh (3)
12-16: Improve error message clarity.The error message could be more descriptive about what the script expects as arguments.
Consider rephrasing to include more details about expected arguments.
20-23: Clarify error message for network validation.The error message should explicitly mention that only 'gamma' and 'omega' are valid networks.
Consider rephrasing for clarity.
143-148: Improve error handling for file existence check.The script should handle the case where the specified file does not exist more robustly.
Consider adding more detailed error handling or logging.
jterzis
changed the title
jterzis
changed the base branch from
main
to
jt/net-80-upgrade-diff-report-render
jterzis
changed the title
NET-80 - splits out bytecode-diff improvements from #1008 (originally kept them there to test github action) into this pr, leaving that pr solely focused on the ci implementation of the bytecode-diff e2e system. In this pr: - `upgrade-facets.sh` bash script that takes a diff report as input and runs the appropriate make target from /contracts depending on the environment attempting to upgrade facets that show a diff in the yaml report. - `new command add-hashes` in bytecode-diff that fetches remote bytecode from output of `upgrade-facets.sh` and adds keccak256 hash to combined report. Report is then rendered in html using a go based html template. - split out diffing tool commands into separate package from run_util to simplify main.go.
Full implementation of:
Pipeline steps:
gamma-reportsserving latest report onhttps://gamma-reports.river.build