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

add remove peer #134

Merged
merged 5 commits into from
Aug 20, 2013
Merged

add remove peer #134

merged 5 commits into from
Aug 20, 2013

Conversation

xiang90
Copy link
Contributor

@xiang90 xiang90 commented Aug 19, 2013

No description provided.

@xiang90
Copy link
Contributor Author

xiang90 commented Aug 19, 2013

TODO: tests


// The name of the remove command in the log
func (c *RemoveCommand) CommandName() string {
return "etcd:remove"
Copy link
Contributor

Choose a reason for hiding this comment

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

can you use the commandName func?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@philips Sure. will change.

@philips
Copy link
Contributor

philips commented Aug 19, 2013

lgtm after cleaning up the function signature on getMachines and some of the comments.

@philips
Copy link
Contributor

philips commented Aug 19, 2013

lgtm, thanks for cleaning that up.

@@ -140,18 +142,25 @@ func (c *JoinCommand) CommandName() string {
// Join a server to the cluster
func (c *JoinCommand) Apply(raftServer *raft.Server) (interface{}, error) {

if c.Name == r.Name() {
Copy link
Contributor

Choose a reason for hiding this comment

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

could you write a comment explaining this, I know why this works but not everyone reading the code will.

@philips
Copy link
Contributor

philips commented Aug 19, 2013

lgtm besides the error message.

xiang90 added a commit that referenced this pull request Aug 20, 2013
@xiang90 xiang90 merged commit 41b2175 into etcd-io:master Aug 20, 2013
tjungblu pushed a commit to tjungblu/etcd that referenced this pull request Sep 5, 2022
…nshift-4.12-ose-etcd

Updating ose-etcd images to be consistent with ART
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants