-
Notifications
You must be signed in to change notification settings - Fork 10.2k
Clean up the use of static path in Antithesis #20817
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: main
Are you sure you want to change the base?
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nwnt The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted filessee 29 files with indirect coverage changes @@ Coverage Diff @@
## main #20817 +/- ##
==========================================
+ Coverage 69.13% 69.20% +0.07%
==========================================
Files 422 422
Lines 34824 34824
==========================================
+ Hits 24075 24100 +25
+ Misses 9349 9326 -23
+ Partials 1400 1398 -2 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
@nwnt, it looks like your branch is out of date and it needs a rebase, as it is showing changes I did to the |
|
|
||
| // CheckHealth checks health of all etcd nodes | ||
| func CheckHealth() bool { | ||
| cfg := common.MakeConfig(NodeCount) |
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.
Can you split the PR?
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.
Hmmm this PR is trying to remove the dependency on the CFG_NODE_COUNT build-time flag and its references such as those in the code, the make file, and docker compose files. Most of the changes are related to that removal. Do you want me to try breaking the PR further? I'm thinking maybe I'll just start with the code part, then docker compose and then the make file. Does that sound ok to you?
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.
Please focus on making it reviewable.
Signed-off-by: Nont <nont@duck.com>
9fd7508 to
4bcb2ea
Compare
|
@nwnt: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Continuation of #20736, and should be the last one. This fully migrates to solely using the environment variables instead of the fixed config.
@serathius, @joshjms