-
-
Notifications
You must be signed in to change notification settings - Fork 290
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
chore: update flag to pass builder url #6190
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## unstable #6190 +/- ##
=========================================
Coverage 90.35% 90.35%
=========================================
Files 78 78
Lines 8087 8087
Branches 490 490
=========================================
Hits 7307 7307
Misses 772 772
Partials 8 8 |
Performance Report✔️ no performance regression detected Full benchmark results
|
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 is a breaking change in cli usage and i don't actually see real need except may be clarifying help on this
It's not breaking as I kept |
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 as previous option still works as alias
🎉 This PR is included in v1.13.0 🎉 |
* Update flag to pass builder url * Throw error if multiple builder urls are set
Motivation
Lodestar only supports a single builder URL and silently removes fallback URLs if multiple are provided. This behavior is not transparent to the end user and we should be more explicit about it.
Description
Update flag to pass builder URL to
--builder.url
(previous flag is kept as alias)Note that I explicitly removed the support for array and comma-separated values as I think it is better to throw an error on startup then to let the user have wrong assumptions that builder has a fallback configured. I don't know any setup that sets multiple builders but better to have an informative error.
Closes #6181