Skip to content

Detect CI from environment#201

Merged
3v0k4 merged 9 commits intomasterfrom
detect-ci
Jun 7, 2023
Merged

Detect CI from environment#201
3v0k4 merged 9 commits intomasterfrom
detect-ci

Conversation

@3v0k4
Copy link
Copy Markdown
Contributor

@3v0k4 3v0k4 commented Jun 6, 2023

Detect CI from environment and get the correct ENVs instead of trying all of them and risk conflicts.

Fixes https://trello.com/c/duyuldsW/76-detect-ci-first-before-reading-the-env-vars

@3v0k4 3v0k4 self-assigned this Jun 6, 2023
@3v0k4 3v0k4 marked this pull request as ready for review June 6, 2023 14:13
@3v0k4 3v0k4 requested a review from ArturT June 6, 2023 14:13
Copy link
Copy Markdown
Member

@ArturT ArturT left a comment

Choose a reason for hiding this comment

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

@3v0k4 Nice work. I added a few suggestions to simplify the code base.

def ci_env_for(env_name)
value = nil
ci_list = KnapsackPro::Config::CI.constants - [:Base, :GitlabCI]
# load GitLab CI first to avoid edge case with order of loading envs for CI_NODE_INDEX
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice that we solve this ugly order workaround for GitLab CI.

Comment thread lib/knapsack_pro/config/ci/base.rb Outdated
Comment thread lib/knapsack_pro/config/ci/buildkite.rb Outdated
Comment thread lib/knapsack_pro/config/ci/circle.rb Outdated
end

def detected
ENV['CI_NAME'] == 'codeship' ? self.class : nil
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is ok. Should stay as it is. The key CI_NAME does not include the CI provider name so we can't assume what is the CI provider. We need to check the value is codeship.

Comment thread lib/knapsack_pro/config/ci/gitlab_ci.rb Outdated
Comment thread lib/knapsack_pro/config/ci/semaphore2.rb Outdated
Comment thread lib/knapsack_pro/config/ci/semaphore2.rb
Comment thread lib/knapsack_pro/config/ci/travis.rb Outdated
3v0k4 and others added 8 commits June 7, 2023 11:51
Co-authored-by: Artur Trzop <arturtrzop@gmail.com>
Co-authored-by: Artur Trzop <arturtrzop@gmail.com>
Co-authored-by: Artur Trzop <arturtrzop@gmail.com>
Co-authored-by: Artur Trzop <arturtrzop@gmail.com>
Co-authored-by: Artur Trzop <arturtrzop@gmail.com>
Co-authored-by: Artur Trzop <arturtrzop@gmail.com>
Co-authored-by: Artur Trzop <arturtrzop@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants