Skip to content

Conversation

@ransombriggs
Copy link
Contributor

On #146 I was getting spec failures because the locked version of minitest does not work with rails 3.1.1, so bumping to one that should work: https://github.com/seattlerb/minitest/blob/master/History.rdoc#5143--2021-01-05-

minitest-5.[14](https://github.com/mcmire/super_diff/runs/5875858335?check_suite_focus=true#step:5:14).2 requires ruby version < 3.1, >= 2.2, which is incompatible with the current version, ruby 3.1.1p18 Error: Process completed with exit code 5. 
@ransombriggs
Copy link
Contributor Author

The nokogiri build failed because the net/ftp gem is missing, added it to all rails gemfiles in 5d51345

`require': cannot load such file -- net/ftp (LoadError) 
Copy link
Collaborator

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

All of the files in gemfiles/* are generated by a gem called Appraisal. There is "master list" kept in the Appraisals file. You can see that it's sort of a gemfile of gemfiles, where a set of gems is grouped by Rails and RSpec version. So, if you want to add a new gem, it's better to add it to one or more groups instead. Then you can run appraisal install on the command line to install all gems in all groups.

@ransombriggs
Copy link
Contributor Author

Thanks for the tip on appraisal, I have not used that before, added properly in 37700de

@ransombriggs
Copy link
Contributor Author

Could you run the workflow again to see if this fixes the build?

@ransombriggs
Copy link
Contributor Author

It turns out that you cannot use net-ftp in ruby < 2.6 so I had to bump appraisal to a ref version from github to get the install_if functionality which allowed me to conditionally require the gem in certain versions of ruby, see fab85ef

net-protocol-0.1.3 requires ruby version >= 2.6.0, which is incompatible with the current version, ruby 2.5.9p229 Error: Process completed with exit code 5. 
@ransombriggs
Copy link
Contributor Author

This passed finally it looks like, could you merge this and I can bring it into the other PRs?

@mcmire mcmire merged commit 48e6930 into splitwise:master Apr 14, 2022
@mcmire
Copy link
Collaborator

mcmire commented Apr 14, 2022

TIL about install_if! That could reduce the need for extra appraisals... 💭

@ransombriggs ransombriggs deleted the bump-minitest-version branch April 14, 2022 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants