Skip to content

Remove CircleCI#6671

Merged
mitchellhenke merged 2 commits intomainfrom
mitchellhenke/remove-circle-ci
Aug 18, 2022
Merged

Remove CircleCI#6671
mitchellhenke merged 2 commits intomainfrom
mitchellhenke/remove-circle-ci

Conversation

@mitchellhenke
Copy link
Contributor

@mitchellhenke mitchellhenke commented Aug 1, 2022

With #6614 live, we should be able to drop CircleCI if we want 🙂

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/remove-circle-ci branch 4 times, most recently from 133425e to 9cf91bc Compare August 1, 2022 18:59
@mitchellhenke mitchellhenke marked this pull request as ready for review August 1, 2022 19:25
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

🌶️ ooo spicy

@aduth
Copy link
Contributor

aduth commented Aug 2, 2022

There's a handful of other references to CircleCI, should we clean those up as well?

$ ag circleci -i --ignore-dir=node_modules
config/environments/test.rb
39:  # Disable lograge when computing coverage and not in CircleCI, where lograge is required.

spec/simplecov_helper.rb
64:      # CircleCI uses JSON formatting for CodeClimate

README.md
4:[![Build Status](https://circleci.com/gh/18F/identity-idp.svg?style=svg)](https://circleci.com/gh/18F/identity-idp)
$ ag circle_ci -i --ignore-dir=node_modules
spec/simplecov_helper.rb
63:    elsif ENV['CIRCLE_CI']

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, is the intention to keep this around? I would have expected the config to be removed altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. The real step is to click the big red button on the CircleCI website, I just wanted this to be green for now since CircleCI complained when I deleted the file.

changelog: Internal, Continuous Integration, Remove CircleCI
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/remove-circle-ci branch from 9cf91bc to 9583647 Compare August 16, 2022 15:46
@mitchellhenke
Copy link
Contributor Author

There's a handful of other references to CircleCI, should we clean those up as well?

$ ag circleci -i --ignore-dir=node_modules
config/environments/test.rb
39:  # Disable lograge when computing coverage and not in CircleCI, where lograge is required.

spec/simplecov_helper.rb
64:      # CircleCI uses JSON formatting for CodeClimate

README.md
4:[![Build Status](https://circleci.com/gh/18F/identity-idp.svg?style=svg)](https://circleci.com/gh/18F/identity-idp)
$ ag circle_ci -i --ignore-dir=node_modules
spec/simplecov_helper.rb
63:    elsif ENV['CIRCLE_CI']

thank you for catching, deleted these

@mitchellhenke mitchellhenke merged commit a1c9177 into main Aug 18, 2022
@mitchellhenke mitchellhenke deleted the mitchellhenke/remove-circle-ci branch August 18, 2022 15:07
Comment on lines -39 to -41
# Disable lograge when computing coverage and not in CircleCI, where lograge is required.
# This enables scanning for view test coverage with `rake test:scan_log_for_view_coverage`
config.lograge.enabled = !ENV['COVERAGE'] || ENV['CI']
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to confirm, should we still be enabling this given the other half of the condition?

Suggested change
# Disable lograge when computing coverage and not in CircleCI, where lograge is required.
# This enables scanning for view test coverage with `rake test:scan_log_for_view_coverage`
config.lograge.enabled = !ENV['COVERAGE'] || ENV['CI']
# Disable lograge when computing coverage, where lograge is required.
# This enables scanning for view test coverage with `rake test:scan_log_for_view_coverage`
config.lograge.enabled = !ENV['COVERAGE']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't impact the current coverage calculation (https://gitlab.login.gov/lg/identity-idp/-/jobs/51849), so it seems ok for now.

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.

4 participants