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 against released Rails #224

Merged
merged 4 commits into from
Jan 9, 2024
Merged

Conversation

andyw8
Copy link
Contributor

@andyw8 andyw8 commented Jan 9, 2024

As discussed in #220

@andyw8 andyw8 force-pushed the andyw8/test-against-released-rails branch from 634603b to b812711 Compare January 9, 2024 17:12
@andyw8 andyw8 marked this pull request as ready for review January 9, 2024 18:36
@andyw8 andyw8 requested a review from a team as a code owner January 9, 2024 18:36
ruby: ["3.0", "3.1", "3.2", "3.3", "head"]
include:
- ruby: "head"
experimental: true
- gemfile: "gemfiles/Gemfile-rails-main"
experimental: true
exclude:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid the headache of Bundler platform changes.

@@ -1,4 +1,4 @@
class CreatePosts < ActiveRecord::Migration[7.2]
class CreatePosts < ActiveRecord::Migration[7.1]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran a search and replace for 7.2 to 7.1.

@@ -9,13 +9,21 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
gemfile:
- Gemfile
- gemfiles/Gemfile-rails-main
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed the same naming as in tapioca.

@andyw8 andyw8 requested a review from vinistock January 9, 2024 18:42
@andyw8
Copy link
Contributor Author

andyw8 commented Jan 9, 2024

(The branch protection rules will need updated once this is merged - we will need to bypass them for now).

@@ -0,0 +1,22 @@
# frozen_string_literal: true

source "https://rubygems.org"
Copy link
Member

Choose a reason for hiding this comment

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

Would it work if we used eval_gemfile to keep this one in sync with the top level? Or does it complain about rails appearing twice?

eval_gemfile("../Gemfile")
gem "rails", github: "rails/rails", branch: "main"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's complains:

There was an error parsing `Gemfile-rails-main`: You cannot specify the same gem twice coming from different sources.

but we could extract out the commonality, and call eval_gemfile from each?

(@Morriar do you have some approach for keeping these in sync on Tapioca?)

Copy link
Member

Choose a reason for hiding this comment

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

In that case, I think it should probably be fine. We can always re-evaluate if it becomes difficult to maintain.

Another option would be using an environment variable to decide only the rails dependency.

if ENV["USE_RAILS_MAIN"]
  gem "rails", github: "rails/rails", branch: "main"
else
  gem "rails"
end

@andyw8 andyw8 force-pushed the andyw8/test-against-released-rails branch from 1954a2e to a4bff73 Compare January 9, 2024 19:57
@andyw8 andyw8 enabled auto-merge January 9, 2024 19:58
@andyw8 andyw8 merged commit a350bba into main Jan 9, 2024
92 of 116 checks passed
@andyw8 andyw8 deleted the andyw8/test-against-released-rails branch January 9, 2024 20:19
@andyw8
Copy link
Contributor Author

andyw8 commented Jan 9, 2024

I've updated the branch protection rules.

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.

None yet

2 participants