-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix drop host from a zone #2877
Conversation
bea2eed
to
5c39d87
Compare
please wait for a minute ... |
5c39d87
to
0064080
Compare
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.
Ready? Generally LGTM, add some test if possible
015bd85
to
110d36a
Compare
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.
Good job
Close #2846 |
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.
Good job. LGTM.
how about a zone with only one host (no parts), could it be dropped? |
In my opinion, it's OK |
it's ok for me too, just confirmation. |
@darionyaphet please fix the format of this PR. you can use following commands to do that: $ cd /path/to/build/directory
$ make clang-format |
thanks @yixinglu but it looks like there's no such command?
By the way, only update branch will cause the format check failed? |
2622dfa
f0ac932
to
2622dfa
Compare
2622dfa
to
bdbfc08
Compare
Append the path |
No description provided.