Skip to content

Fix environment variables for proofer gems#977

Merged
pkarman merged 1 commit intomasterfrom
pek-proofers
Jan 23, 2017
Merged

Fix environment variables for proofer gems#977
pkarman merged 1 commit intomasterfrom
pek-proofers

Conversation

@pkarman
Copy link
Contributor

@pkarman pkarman commented Jan 23, 2017

Why: Vendor-specific drivers all expect UPPER case env vars.

Copy link
Contributor

@jessieay jessieay left a comment

Choose a reason for hiding this comment

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

One suggestion. Overall, this makes me very happy, as I am also used to env vars being all caps all the time

Copy link
Contributor

Choose a reason for hiding this comment

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

what this does is pretty straightforward, but a test might be good just as a form of documentation. I can imagine someone trying to add a lowercase env var and not knowing why it is always getting uppercased

@pkarman
Copy link
Contributor Author

pkarman commented Jan 23, 2017

thanks @jessieay -- I refactored the initializer code into Idv::Vendor and added a test.

Copy link
Contributor

@jessieay jessieay left a comment

Choose a reason for hiding this comment

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

Code climate isn't pleased, but I am!

**Why**: Vendor-specific drivers all expect UPPER case env vars.
@pkarman pkarman merged commit 2bfed9f into master Jan 23, 2017
@pkarman pkarman deleted the pek-proofers branch January 23, 2017 18:15
amoose pushed a commit that referenced this pull request Feb 7, 2017
**Why**: Vendor-specific drivers all expect UPPER case env vars.
amoose pushed a commit that referenced this pull request Feb 24, 2017
**Why**: Vendor-specific drivers all expect UPPER case env vars.
amoose pushed a commit that referenced this pull request Feb 28, 2017
**Why**: Vendor-specific drivers all expect UPPER case env vars.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants