-
Notifications
You must be signed in to change notification settings - Fork 3k
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
go: update to urfave v2, merge endpoint-monitor and op-heartbeat into main go module #6098
Conversation
|
✅ Deploy Preview for opstack-docs canceled.
|
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.
I think the flipped CheckRequired is the reason the tests are failing.
Lots of code to review, but it was pretty straightforwards.
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.
Generally looks good to me
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #6098 +/- ##
===========================================
+ Coverage 44.72% 45.46% +0.73%
===========================================
Files 304 434 +130
Lines 23290 28002 +4712
Branches 0 688 +688
===========================================
+ Hits 10417 12731 +2314
- Misses 11914 14237 +2323
- Partials 959 1034 +75
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This PR has been added to the merge queue, and will be merged soon. |
This PR is next in line to be merged, and will be merged as soon as checks pass. |
1 similar comment
This PR is next in line to be merged, and will be merged as soon as checks pass. |
Description
Cannon + Geth are already using urfave CLI v2.
With this upgrade, in separate later PRs, we can improve op-service without complicating a later v2 migration, and utilize the improved context/shutdown functionality of urfave CLI v2.
In practice, most of the v2 CLI migration changes are:
Tests
No functionality changes
Metadata
Fix CLI-3946