Skip to content
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

Closed
wants to merge 2 commits into from

Conversation

cpfergus1
Copy link
Contributor

Summary

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

  • I have written a thorough PR description.
  • I have kept my commits small and atomic.
  • I have used clear, explanatory commit messages.

The following are not always needed (cross them out if they are not):

  • I have added automated tests to cover my changes.
  • I have attached screenshots to demo visual changes.
  • I have opened a PR to update the guides.
  • I have updated the readme to account for my changes.

@cpfergus1 cpfergus1 force-pushed the nebulab/refactor-ci branch 8 times, most recently from 3026098 to 1e2df19 Compare September 30, 2022 22:45
@codecov
Copy link

codecov bot commented Sep 30, 2022

Codecov Report

Merging #4652 (1e2df19) into master (29da36c) will decrease coverage by 0.43%.
The diff coverage is n/a.

❗ Current head 1e2df19 differs from pull request most recent head a28d718. Consider uploading reports for the commit a28d718 to get more accurate results

@@            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     
Impacted Files Coverage Δ
...pp/models/spree/taxon/active_storage_attachment.rb 0.00% <0.00%> (-100.00%) ⬇️
...oncerns/spree/active_storage_adapter/attachment.rb 0.00% <0.00%> (-92.69%) ⬇️
...erns/spree/active_storage_adapter/normalization.rb 41.17% <0.00%> (-47.06%) ⬇️
...pp/models/concerns/spree/active_storage_adapter.rb 67.79% <0.00%> (-23.73%) ⬇️
...pp/models/spree/image/active_storage_attachment.rb 83.33% <0.00%> (-16.67%) ⬇️
core/lib/spree/rails_compatibility.rb 63.63% <0.00%> (-3.04%) ⬇️
core/lib/spree/testing_support/dummy_app.rb 94.73% <0.00%> (-2.64%) ⬇️
core/app/models/spree/credit_card.rb 96.82% <0.00%> (+0.05%) ⬆️
...ore/app/models/spree/image/paperclip_attachment.rb 91.66% <0.00%> (+41.66%) ⬆️
...ore/app/models/spree/taxon/paperclip_attachment.rb 100.00% <0.00%> (+100.00%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@elia elia left a 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

Comment on lines 228 to 230
rails_version:
type: string
default: "~> 7.0.1"
Copy link
Member

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

Comment on lines 222 to 233
ex:
type: string
default: postgres
Copy link
Member

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
Comment on lines 44 to 49
gem 'net-http', require: false
end

gem 'net-smtp', require: false
gem 'net-imap', require: false
gem 'net-pop', require: false
Copy link
Member

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?

@cpfergus1 cpfergus1 force-pushed the nebulab/refactor-ci branch 7 times, most recently from ac70b6e to a28d718 Compare October 4, 2022 13:52
@cpfergus1 cpfergus1 changed the title Nebulab/refactor ci Test matrix of ruby/rails versions on different Databases Oct 4, 2022
@elia elia force-pushed the nebulab/refactor-ci branch from a28d718 to 0058e83 Compare October 10, 2022 09:34
@elia elia force-pushed the nebulab/refactor-ci branch from 9e054d1 to a134be3 Compare October 10, 2022 11:03
@elia
Copy link
Member

elia commented Oct 10, 2022

Replaced by #4666

@elia elia closed this Oct 10, 2022
@elia elia deleted the nebulab/refactor-ci branch October 10, 2022 12:58
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