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 new setting to determine if running in CI env #3672

Merged
merged 1 commit into from Oct 27, 2017

Conversation

Projects
None yet
5 participants
@RomanIakovlev
Contributor

RomanIakovlev commented Oct 26, 2017

Fixes #3653

@eed3si9n eed3si9n added the ready label Oct 26, 2017

@Duhemm

The changes LGTM, but I'm wondering if there's a risk of people having troubles with timeouts in Travis (and potentially other CIs) because no output may be produced for a relatively long time. Can people remove the BUILD_NUMBER or CI environment variable when running outside of self-hosted CIs? I would suggest to:

  • If BUILD_NUMBER or CI is defined, log a line saying that logging can be explicitly enabled by setting an environment variable
  • Check if that environment variable is set, and if so, still log the resolving information.

@jvican What do you think?

Show outdated Hide outdated main/src/main/scala/sbt/Defaults.scala Outdated
@jvican

jvican approved these changes Oct 26, 2017

I think this looks awesome! Using sys.env.contains is indeed better 😄. Thanks for the hard work @RomanIakovlev

@Duhemm

Duhemm approved these changes Oct 26, 2017

@jvican Duh! You didn't change that. LGTM, thanks and congrats! 🎉

Show outdated Hide outdated main/src/main/scala/sbt/Defaults.scala Outdated
@RomanIakovlev

This comment has been minimized.

Show comment
Hide comment
@RomanIakovlev

RomanIakovlev Oct 26, 2017

Contributor

Moved to globalSbtCore, replaced .get(...).isDefined with contains.

Contributor

RomanIakovlev commented Oct 26, 2017

Moved to globalSbtCore, replaced .get(...).isDefined with contains.

@dwijnand

Thank you!

@RomanIakovlev

This comment has been minimized.

Show comment
Hide comment
@RomanIakovlev

RomanIakovlev Oct 27, 2017

Contributor

@Duhemm regarding your concern about timeouts because of no output produced, if I understood it correctly. I just wanted to point this PR doesn't change the actual behavior of sbt, because the code to check the environment variables in ivyLoggingLevel already used to do the very same check.

Contributor

RomanIakovlev commented Oct 27, 2017

@Duhemm regarding your concern about timeouts because of no output produced, if I understood it correctly. I just wanted to point this PR doesn't change the actual behavior of sbt, because the code to check the environment variables in ivyLoggingLevel already used to do the very same check.

@dwijnand dwijnand added this to the 1.1.0 milestone Oct 27, 2017

@dwijnand dwijnand merged commit bb6d8d6 into sbt:1.x Oct 27, 2017

2 checks passed

codacy/pr Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dwijnand dwijnand removed the ready label Oct 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment