Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Introducing test constants into separate files #1272

Conversation

shuklanirdesh82
Copy link
Contributor

fixes #1271

Testing done: Yes locally

PASS
2017/05/17 04:17:53 -----Clean-up finished - current time:  2017-05-17 04:17:53.127831766 +0000 UTC
ok  	github.com/vmware/docker-volume-vsphere/tests/e2e	125.418s

ListVolumes = dockerVol + "ls "

// Inspect to grab volume properties
Inspect = dockerVol + "inspect "
Copy link
Contributor

Choose a reason for hiding this comment

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

Inspect => InspectVolume

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Inspect = dockerVol + "inspect "

// Remove constant refers delete volume command
Remove = dockerVol + "rm "
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove => RemoveVolume

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

package dockercli

const (
docker = "docker "
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: why these 2 names start with small case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kept it private members ... no need to export them

Copy link
Contributor

Choose a reason for hiding this comment

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

Then above VmdkopsAdmin and VmdkopsAdminVolume should not be exported either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, updating into the next diff.

Copy link
Contributor

@shaominchen shaominchen left a comment

Choose a reason for hiding this comment

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

LGTM.

@shuklanirdesh82 shuklanirdesh82 merged commit ae912ee into vmware-archive:master May 17, 2017
@shuklanirdesh82 shuklanirdesh82 deleted the testConstants.shuklanirdesh82 branch May 17, 2017 20:51
@shuklanirdesh82 shuklanirdesh82 removed this from the 0.15 milestone May 25, 2017
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.

Separate out all constants to its dedicated package to reuse
3 participants