Skip to content
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

Merged
merged 1 commit into from
Sep 23, 2021
Merged

Conversation

darionyaphet
Copy link
Contributor

No description provided.

@darionyaphet
Copy link
Contributor Author

please wait for a minute ...

Copy link
Contributor

@critical27 critical27 left a 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

critical27
critical27 previously approved these changes Sep 18, 2021
Copy link
Contributor

@critical27 critical27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job

@critical27
Copy link
Contributor

Close #2846

Copy link
Contributor

@bright-starry-sky bright-starry-sky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job. LGTM.

@HarrisChu
Copy link
Contributor

HarrisChu commented Sep 23, 2021

how about a zone with only one host (no parts), could it be dropped?

@darionyaphet
Copy link
Contributor Author

how about a zone with only one host (no parts), could it be dropped?

In my opinion, it's OK

@HarrisChu
Copy link
Contributor

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.
if it's ok, I will add more cases, e.g. add group via a zone without any hosts.

@yixinglu
Copy link
Contributor

@darionyaphet please fix the format of this PR. you can use following commands to do that:

$ cd /path/to/build/directory
$ make clang-format

@darionyaphet
Copy link
Contributor Author

darionyaphet commented Sep 23, 2021

@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?

-- Build files have been written to: /data/darion/work/nebula/build
xargs: clang-format: No such file or directory
make[3]: *** [CMakeFiles/clang-format.dir/build.make:70: CMakeFiles/clang-format] Error 127
make[2]: *** [CMakeFiles/Makefile2:2295: CMakeFiles/clang-format.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:2302: CMakeFiles/clang-format.dir/rule] Error 2
make: *** [Makefile:264: clang-format] Error 2

By the way, only update branch will cause the format check failed?

@yixinglu
Copy link
Contributor

@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?

-- Build files have been written to: /data/darion/work/nebula/build
xargs: clang-format: No such file or directory
make[3]: *** [CMakeFiles/clang-format.dir/build.make:70: CMakeFiles/clang-format] Error 127
make[2]: *** [CMakeFiles/Makefile2:2295: CMakeFiles/clang-format.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:2302: CMakeFiles/clang-format.dir/rule] Error 2
make: *** [Makefile:264: clang-format] Error 2

By the way, only update branch will cause the format check failed?

Append the path /opt/vesoft/toolset/clang/10.0.0/bin to the PATH environment variable.

@critical27 critical27 merged commit c2032e4 into vesoft-inc:master Sep 23, 2021
@darionyaphet darionyaphet deleted the fix-drop-host branch September 24, 2021 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants