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

Update env vars to meaningful names #713

Merged
merged 1 commit into from
Jun 20, 2017
Merged

Conversation

nitisht
Copy link
Contributor

@nitisht nitisht commented Jun 16, 2017

  • Changed environment var S3_ADDRESS to SERVER_ENDPOINT
  • Changed environment var S3_SECURE to ENABLE_HTTPS

harshavardhana
harshavardhana previously approved these changes Jun 16, 2017
@harshavardhana harshavardhana requested a review from krisis June 16, 2017 22:53
@krishnasrinivas
Copy link
Contributor

I think it is better to prefix the env vars by MINIO_TEST_

MINIO_TEST_ENDPOINT
MINIO_TEST_ACCESS_KEY
MINIO_TEST_SECRET_KEY
MINIO_TEST_ENABLE_HTTPS

@harshavardhana
Copy link
Member

MINIO_TEST_ENDPOINT
MINIO_TEST_ACCESS_KEY
MINIO_TEST_SECRET_KEY
MINIO_TEST_ENABLE_HTTPS

Perhaps we can use these consistently across all libraries.. 👍

Copy link
Member

@krisis krisis left a comment

Choose a reason for hiding this comment

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

Whatever name we pick for environment variables it is best defined as golang constants. I.e,

const (
serverEndpoint = "SERVER_ENDPOINT",
...
)

@nitisht
Copy link
Contributor Author

nitisht commented Jun 17, 2017

Minio SDKs will run against a target that is S3 compatible. So, I thought adding MINIO/S3 will be redundant.

@krisis yes, that will be better. I will do it

 - Changed environment var S3_SECURE to ENABLE_HTTPS
 - Added environment vars as const
@nitisht
Copy link
Contributor Author

nitisht commented Jun 17, 2017

Added constants

const (
	serverEndpoint = "SERVER_ENDPOINT"
	accessKey      = "ACCESS_KEY"
	secretKey      = "SECRET_KEY"
	enableSecurity = "ENABLE_HTTPS"
)

@harshavardhana harshavardhana merged commit ce8164b into minio:master Jun 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants