deis / workflow-cli

The CLI for Deis Workflow
http://deis.com
MIT License
31 stars 43 forks source link

fix(config): support CRLF config files #229

Closed Joshua-Anderson closed 7 years ago

Joshua-Anderson commented 8 years ago

Sorry I didn't add any tests for this. I'm not sure of any ways to easily test config:pull and config:push because I need to mock os.ModeCharDevice and I don't have the time to investigate it right now. 😢

How to manually test:

Create a .env file with CRLF line endings and...

Before fix:

deis config:push
...
deis run 'printf %q $TEST'
Running 'printf %q $TEST'...
$'123456\r'

After fix:

deis config:push
...
deis run 'printf %q $TEST'
Running 'printf %q $TEST'...
123456
deis-bot commented 8 years ago

@bacongobbler, @mboersma and @aledbf are potential reviewers of this pull request based on my analysis of git blame information. Thanks @Joshua-Anderson!

codecov-io commented 8 years ago

Current coverage is 71.91% (diff: 0.00%)

No coverage report found for master at 05892a7.

Powered by Codecov. Last update 05892a7...03993f7

ultimateboy commented 7 years ago

This PR needed a rebase. Opened a new, rebased PR here: https://github.com/deis/workflow-cli/pull/250

Joshua-Anderson commented 7 years ago

Does anybody else have a chance to review this PR?

kmala commented 7 years ago

This LGTM but i see another PR opened with your commit #250

Joshua-Anderson commented 7 years ago

Yep. Either one works for me! I can LGTM the other one if you'd rather see that one merged.

kmala commented 7 years ago

i LGTM the #250 as it has ci passing thought i don't see jenkins job on it.