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
Ant and jenkins modes #82
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clayreimann I found a few issues, but I might have missed something.
I think it's common practice to give env variables priority over other config as they are more dynamic. But I guess you can go either way
@yonjah I've updated my PR with your comments. The tests are passing, maybe you could take it for a spin in whatever builds you're running to ensure it behaves as you expect? |
Thanks @clayreimann I'll try to do it today |
I ran my tests using this version and couldn't find any issues with the reports. |
And the crickets have it, merging as is. @soryy708, if you find bugs with this implementation open an issue. |
@yonjah I've combined #74 and #75 in this PR. I've somehow broken one of the Jenkins tests, I hope to fix it tomorrow.
I have slightly changed the behavior of your
deduceConfig
function. The search order is nowconfig -> env -> default
instead ofenv -> config -> default
. If you have a good argument for the older behavior, please share it. Otherwise please validate that I've faithfully merged your work in here.Closes #70
Closes #71
Closes #74
Closes #75