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

Replace Appraisals by eval_gemfile #2431

Merged
merged 2 commits into from
Apr 26, 2024

Conversation

ericproulx
Copy link
Contributor

This PR drops Appraisals in favor of eval_gemfile. It also updates some gems like rubocop, rack-test etc ...
I don't know if we need instructions if someone wants to add a new Gemfile but looking at the current ones might give a very good idea of the how to.

Update test gems including rubocop
Regenerate Rubocop's todo
@grape-bot
Copy link

grape-bot commented Apr 16, 2024

1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Generated by 🚫 Danger

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I can live with this :) Some nits below, up to you.

.rubocop_todo.yml Outdated Show resolved Hide resolved
spec/support/chunks.rb Outdated Show resolved Hide resolved
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Actually two tests fail for good reason! Double check that the changes actually don't change the required gems. Probably need to swap the order of things.

Fix rubocop
Add CHANGELOG
@ericproulx ericproulx requested a review from dblock April 17, 2024 19:30
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I spot-checked that we end up with Rack 2.0 for the rack_2.0.gemfile. I think we should add tests in the rack and rails integrations that ensure that the version matches the intended one, e.g.

expect(Gem::Version.new(Rack.release).segments.first).to eq 2

Up to you if you want to do this as part of this/future PR/maybe I can get to it, too.

@dblock dblock merged commit 96c2a91 into ruby-grape:master Apr 26, 2024
45 checks passed
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.

3 participants