Skip to content
This repository has been archived by the owner on Jul 6, 2023. It is now read-only.

Conversation

raghavendra-talur
Copy link
Member

Signed-off-by: Raghavendra Talur rtalur@redhat.com

Signed-off-by: Raghavendra Talur <rtalur@redhat.com>
@centos-ci
Copy link
Collaborator

Can one of the admins verify this patch?

@@ -28,7 +28,7 @@ const (
EntryStateUnknown EntryState = ""
EntryStateOnline EntryState = "online"
EntryStateOffline EntryState = "offline"
EntryStateFailed EntryState = "failed"
EntryStateFailed EntryState = "removed"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also change the variable throughout the code EntryStateFailed to EntryStateRemoved for completeness

Copy link
Member Author

Choose a reason for hiding this comment

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

@lpabon I realised now that EntryStateFailed is a api. Do you really want to change it? It will break backward compatibility.

Copy link
Contributor

@lpabon lpabon Jul 27, 2017

Choose a reason for hiding this comment

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

True. I think for that reason, this PR cannot be accepted. (API: https://github.com/heketi/heketi/blob/master/pkg/glusterfs/api/types.go#L31 ).

Instead we can always change the string shown in the CLI

@humblec
Copy link
Contributor

humblec commented Jul 25, 2017

@heketi/dev Please review

@@ -28,7 +28,7 @@ const (
EntryStateUnknown EntryState = ""
EntryStateOnline EntryState = "online"
EntryStateOffline EntryState = "offline"
EntryStateFailed EntryState = "failed"
EntryStateFailed EntryState = "removed"
Copy link
Contributor

@lpabon lpabon Jul 27, 2017

Choose a reason for hiding this comment

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

True. I think for that reason, this PR cannot be accepted. (API: https://github.com/heketi/heketi/blob/master/pkg/glusterfs/api/types.go#L31 ).

Instead we can always change the string shown in the CLI

@lpabon
Copy link
Contributor

lpabon commented Jul 31, 2017

Closing this PR since it would change the existing API.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants