-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Test matrix of ruby/rails versions on different Databases #4652
Conversation
3026098
to
1e2df19
Compare
Codecov Report
@@ Coverage Diff @@
## master #4652 +/- ##
==========================================
- Coverage 86.02% 85.59% -0.44%
==========================================
Files 571 571
Lines 14611 14629 +18
==========================================
- Hits 12569 12521 -48
- Misses 2042 2108 +66
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Early review, hope it's useful
.circleci/config.yml
Outdated
rails_version: | ||
type: string | ||
default: "~> 7.0.1" |
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.
nit: I think could accept just 7.0
and the ~>
be added when setting the ENV var below, this would make it look a bit more generic and also prettify the generated job name
.circleci/config.yml
Outdated
ex: | ||
type: string | ||
default: postgres |
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.
Should this parameter be named database
instead?
Probably just me but I had to read twice in order to understand the reference to executors
Gemfile
Outdated
gem 'net-http', require: false | ||
end | ||
|
||
gem 'net-smtp', require: false | ||
gem 'net-imap', require: false | ||
gem 'net-pop', require: false |
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.
Should these be moved up together with net-http
so they can share the guard and comment?
ac70b6e
to
a28d718
Compare
a28d718
to
0058e83
Compare
9e054d1
to
a134be3
Compare
Replaced by #4666 |
Summary
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed (
cross them outif they are not):