Skip to content

Commit

Permalink
Update CI detection cases
Browse files Browse the repository at this point in the history 
PR-URL: #655
Credit: @isaacs
Close: #655
Reviewed-by: @mikemimik
  • Loading branch information
@isaacs
isaacs authored and Michael Perrotte committed Jan 23, 2020
1 parent 7852c0c commit 7dbb914
Showing 1 changed file with 22 additions and 16 deletions.
38 changes: 22 additions & 16 deletions lib/npm.js
View file
Expand Up @@ -283,22 +283,28 @@
ua = ua.replace(/\{arch\}/gi, process.arch)

// continuous integration platforms
const ci = process.env.GERRIT_PROJECT ? 'ci/gerrit'
: process.env.GITLAB_CI ? 'ci/gitlab'
: process.env.CIRCLECI ? 'ci/circle-ci'
: process.env.SEMAPHORE ? 'ci/semaphore'
: process.env.DRONE ? 'ci/drone'
: process.env.GITHUB_ACTION ? 'ci/github-actions'
: process.env.TDDIUM ? 'ci/tddium'
: process.env.JENKINS_URL ? 'ci/jenkins'
: process.env['bamboo.buildKey'] ? 'ci/bamboo'
: process.env.GO_PIPELINE_NAME ? 'ci/gocd'
// codeship and a few others
: process.env.CI_NAME ? `ci/${process.env.CI_NAME}`
// test travis last, since many of these mimic it
: process.env.TRAVIS ? 'ci/travis-ci'
: process.env.CI === 'true' || process.env.CI === '1' ? 'ci/custom'
: ''
const ciName = process.env.GERRIT_PROJECT ? 'gerrit'
: process.env.GITLAB_CI ? 'gitlab'
: process.env.APPVEYOR ? 'appveyor'
: process.env.CIRCLECI ? 'circle-ci'
: process.env.SEMAPHORE ? 'semaphore'
: process.env.DRONE ? 'drone'
: process.env.GITHUB_ACTION ? 'github-actions'
: process.env.TDDIUM ? 'tddium'
: process.env.JENKINS_URL ? 'jenkins'
: process.env['bamboo.buildKey'] ? 'bamboo'
: process.env.GO_PIPELINE_NAME ? 'gocd'
// codeship and a few others
: process.env.CI_NAME ? process.env.CI_NAME
// test travis after the others, since several CI systems mimic it
: process.env.TRAVIS ? 'travis-ci'
// aws CodeBuild/CodePipeline
: process.env.CODEBUILD_SRC_DIR ? 'aws-codebuild'
: process.env.CI === 'true' || process.env.CI === '1' ? 'custom'
// Google Cloud Build - it sets almost nothing
: process.env.BUILDER_OUTPUT ? 'builder'
: false
const ci = ciName ? `ci/${ciName}` : ''
ua = ua.replace(/\{ci\}/gi, ci)

config.set('user-agent', ua.trim())
Expand Down

5 comments on commit 7dbb914

@lawwantsin
Copy link

Choose a reason for hiding this comment

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

my god, what a fine club to be in.

@glensc
Copy link

@glensc glensc commented on 7dbb914 Feb 4, 2020

Choose a reason for hiding this comment

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

what a terrible nesting, I would this refactor to define some map of env variables => ci name, and iterate over the map.

@isaacs
Copy link
Contributor Author

@isaacs isaacs commented on 7dbb914 Feb 5, 2020

Choose a reason for hiding this comment

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

@glensc It's refactored away entirely into @npmcli/ci-detect, but we can't bundle scoped modules into the CLI until v7, due to a bug in how npm v3 handled bundled scoped deps. (npm v3 ships with Node v6, which is the oldest Node.js version supported by npm v6).

@glensc
Copy link

@glensc glensc commented on 7dbb914 Feb 5, 2020

Choose a reason for hiding this comment

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

but still same code there :)

@isaacs
Copy link
Contributor Author

@isaacs isaacs commented on 7dbb914 Feb 5, 2020

Choose a reason for hiding this comment

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

But without standard's abusive indentation.

A series of ternaries is ideal for this use case, because it is the formulation that affords the minimum amount of flexibility. When you see a start of a ternary, you know that the only possible use is (conditional expression) ? (expression) : (expression). There's no possibility for it to have multiple non-expression statements. It cannot throw, return, fail to set the initially assigned variable, etc.

A good rule of thumb in programming is to always use the code form that grants as much flexibility as needed, but no more, so that a reader can "switch off" scanning for other possible behaviors. This is why types are good (and really, imo, the only relevant reason why types are good), why arrow functions and classes are better than function, why brace-less arrow functions and if/else are better than braced, etc.

Keep your flexibility small, so that the intention of a block of code is clear. This is, in the long run, a much more useful and pragmatic rubric than "make it easy to read", because what is "easy to read" depends more on subjective personal experience and the whims of fashion than anything else. Your entire future will be spent as a person with more experience than you have today; optimize for that person, not this one.

Please sign in to comment.