-
-
Notifications
You must be signed in to change notification settings - Fork 530
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 support for CircleCI 2.0 #934
Conversation
And update automatic staging app deployment config
This is not necessary anymore with CircleCI 2.0. Note also that `CIRCLE_ARTIFACTS` isn't defined in 2.0 https://circleci.com/docs/2.0/migration/#tips-for-migrating-from-10-to-20 The default directory `coverage` is added to stored artifacts paths so it shows up on circleci.com `Artifacts` tab.
@@ -177,12 +178,16 @@ def remove_routes_comment_lines | |||
build :remove_routes_comment_lines | |||
end | |||
|
|||
def configure_circleci | |||
say 'Configuring CircleCI' |
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.
Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
lib/suspenders/app_builder.rb
Outdated
def_delegators( | ||
:circleci_adapter, | ||
:configure_circleci, | ||
:configure_circleci_deployment |
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.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
As you can see [in Circle](https://circleci.com/gh/gabebw/hotline-webring), the `circleci-20-test` branches now take ~20 seconds total, while branches still on CircleCI 1.0 (like `master`) took 1 minute and 30 seconds. Thank you to thoughtbot/suspenders#934 and https://circleci.com/sunset1-0/ for a lot of help!
def generate_default | ||
run("spring stop") | ||
generate("suspenders:static") | ||
generate("suspenders:stylesheet_base") | ||
generate("suspenders:testing") | ||
generate("suspenders:ci") |
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.
We'll need to keep this in the generator, so people can remove CI support if they don't want it (or, for people who just add the gem to their Gemfile, so people can add CI support if they want it).
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.
This is starting to look pretty good. As @mike-burns said, we'd like to start moving to put everything in generators so that people can more easily remove things if they don't want it. It's part of an overall direction we're going.
POSTGRES_PASSWORD: "" | ||
steps: | ||
- checkout | ||
|
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.
Give that you have have not spaced out the rest of this file, what are your thoughts on removing this whitespace?
|
||
- restore_cache: | ||
keys: | ||
- <%= app_name %>-{{ .Branch }}-{{ checksum "Gemfile.lock" }} |
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.
Does this need to be branch specific?
I kind of like the idea that this would restore the cache for any ol' branch so long as the gems are the same.
--jobs=3 | ||
|
||
- save_cache: | ||
key: <%= app_name %>-{{ .Branch }}-{{ checksum "Gemfile.lock" }} |
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.
same as above
end | ||
|
||
def configure_circleci_deployment | ||
deploy_command = <<-YML.strip_heredoc |
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.
Is there any reason why this isn't in the the circleci.yml.erb
file?
Since this PR is stale and has some merge conflicts, I'm closing it. We'll use it for reference if needed. Thank you @mehlah! |
CircleCI 1.0 is reaching its EOL this August 31st.
This PR introduce a default configuration for CircleCI 2.0.
I also extracted a
CircleCI
adapter to make things a bit clearer and also because I needed anapp_builder
dependency to be able to useapp_name
in the config template.SimpleCov default artifacts path is added to the stored paths so it shows up in circles.com
Artifacts
tab.Closes #865