-
Notifications
You must be signed in to change notification settings - Fork 332
Rewrite new fork tool to modify blob parameters #1372
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
base: forks/osaka
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1372 +/- ##
============================================
Coverage 94.94% 94.95%
============================================
Files 583 583
Lines 34665 34665
Branches 3070 3070
============================================
+ Hits 32914 32916 +2
+ Misses 1198 1195 -3
- Partials 553 554 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7909e08
to
ebd7090
Compare
|
||
blob_parameters = parser.add_argument_group() | ||
|
||
blob_parameters.add_argument( |
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.
This could get a bit more complicated since post-Osaka, we won't have TARGET_BLOB_GAS_PER_BLOCK as ain independent constant. See this
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.
Should we disable this option post-osaka or allow users to modify it at their own risk?
options = parser.parse_args(args) | ||
|
||
if options.template_fork == "spurious_dragon": | ||
raise NotImplementedError( |
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.
Tangerine Whistle and Dao Fork?
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.
Ugh, if there are more instances of "a self-reference that shouldn't be changed" I don't think I can leave this unimplemented 🤣
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.
Hm, from a quick look, it seems like spurious dragon is the only fork that's referenced by other forks (eg. here).
3e6785e
to
615b3db
Compare
`actions-rs` has been archived and is no longer the recommended way to install packages. Instead, I'm using the Ubuntu-provided `rustup`.
615b3db
to
2347cb8
Compare
What a finicky tool. Should be good to review now. |
What was wrong?
Related to Issue #1357
How was it fixed?
Rewrote the new fork tool to be more than just find-and-replace. It now uses libcst to transform the source somewhat intelligently.
Cute Animal Picture