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 test env and move env helper to be usable everywhere #166

Merged
merged 3 commits into from
Jan 13, 2017
Merged

Conversation

glibsm
Copy link
Collaborator

@glibsm glibsm commented Jan 12, 2017

No description provided.

@mention-bot
Copy link

@glibsm, thanks for your PR! By analyzing the history of the files in this pull request, we identified @anuptalwalkar, @shawnburke and @sectioneight to be potential reviewers.

//
// Useful, for example, to enable alternate functionality of some components
// during testing that might normally require network connectivity or a database
func IsTestEnv() bool {
Copy link

Choose a reason for hiding this comment

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

There could be different environments with different purposes, e.g. staging, internal, etc. Shouldn't test be controlled by the service config as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Based on the earlier discussion, I'll make some changes to this diff and remove this functionality.

Going to keep environmental knowledge restricted
to the config package. Everything else should come
from configuration values, rather than the knowledge
of which environment the application is running in.
@@ -51,7 +51,8 @@ var (
)

var (
_devEnv = "development"
_devEnv = "development"
_testEnv = "test"
Copy link
Contributor

Choose a reason for hiding this comment

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

unused?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

interesting. did go compiler sneeze while processing this line? I thought unused vars were an error

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 92.213% when pulling 1c3822a on is-test into 1da752b on master.

@ascandella
Copy link
Contributor

ascandella commented Jan 13, 2017 via email

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 92.213% when pulling b72efe5 on is-test into 1da752b on master.

@glibsm glibsm merged commit 49970a7 into master Jan 13, 2017
@glibsm glibsm deleted the is-test branch January 13, 2017 23:14
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.

4 participants