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

Some updates! #91

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Some updates! #91

wants to merge 22 commits into from

Conversation

ukd1
Copy link

@ukd1 ukd1 commented Aug 8, 2024

Thanks for writing this, it's exactly what I wanted, and the gem is working nicely in my project. Thanks! 🙏

Apologies for the random / big PR. I wanted use a gem version, instead of a github version, and went down a rabbit hole after I saw that tests for supported ruby weren't really running. These changes:

  • matrix ruby to only currently supported versions
  • matrix appraisal tests as well, but just for ruby 3.3 (could do all, but seems overkill?)
  • add some more tests for path
  • drop test running support for < rails 5.0, and rails 8.0 as it's alpha - more details below
  • add rubocop linting, and updated the code to match the rules github use
  • rewrote some of the URL building stuff to use URI - which uses less raw or html_safe, so should be safer
  • made the github actions build and push when you tag a release (to the hosting accounts gh rubygems - e.g. here)

Re:Rails 8 - alpha dependancies of the actionview aren't published yet, it seems:

The git source https://github.com/rails/rails.git is not yet checked out. Please run `bundle install` before trying to start your application
Fetching https://github.com/rails/rails.git
Fetching gem metadata from https://rubygems.org/...........
Resolving dependencies...
Could not find compatible versions

Because every version of rails depends on actionpack = 8.0.0.alpha
  and actionpack = 8.0.0.alpha could not be found in rubygems repository https://rubygems.org/ or installed locally,
  rails cannot be used.
So, because rails_main.gemfile depends on rails >= 0,
  version solving has failed

Re:Rails - pre-5.0 requires a sub 2.0 bundler, and that was too much to fix:

 canonical-rails % bundle exec appraisal install
>> bundle check --gemfile='/Users/russ/Sites/canonical-rails/gemfiles/rails_4_2.gemfile' || bundle install --gemfile='/Users/russ/Sites/canonical-rails/gemfiles/rails_4_2.gemfile' --retry 1
Bundler can't satisfy your Gemfile's dependencies.
Install missing gems with `bundle install`.
Fetching gem metadata from https://rubygems.org/..........
Resolving dependencies...
Could not find compatible versions

Because the current Bundler version (2.5.6) does not satisfy bundler >= 1.3.0, < 2.0
  and rails >= 4.0.0.beta1, < 5.0.5.rc1 depends on bundler >= 1.3.0, < 2.0,
  rails >= 4.0.0.beta1, < 5.0.5.rc1 cannot be used.
So, because rails_4_2.gemfile depends on rails ~> 4.2.6,
  version solving has failed.

Your bundle requires a different version of Bundler than the one you're running.
Install the necessary version with `gem install bundler:2.0.0.pre.3` and rerun bundler using `bundle _2.0.0.pre.3_ install
--gemfile=/Users/russ/Sites/canonical-rails/gemfiles/rails_4_2.gemfile --retry 1`

@jumph4x
Copy link
Owner

jumph4x commented Aug 8, 2024

@ukd1 thank you for the enthusiasm on this, but this PR is massively non-atomic, so to say.

I'd be much happier reviewing/supporting you on ingesting these changes into the project if we take the time to limit the scope of each PR.

@jumph4x
Copy link
Owner

jumph4x commented Aug 8, 2024

As an additional comment on Rails scope tightening, I'm happy to accept that proposal.

@jumph4x
Copy link
Owner

jumph4x commented Aug 8, 2024

I will aim to review this PR and provide reductions/changes or accept it within a week.

@ukd1
Copy link
Author

ukd1 commented Aug 8, 2024

@jumph4x no worries!

@ukd1
Copy link
Author

ukd1 commented Aug 8, 2024

As an additional comment on Rails scope tightening, I'm happy to accept that proposal.

Ya, tbh, the main reason I didn't "fix" that too was it looked like a pain, and to me pre-rails 5 seems so old. TBH, I think rails 6.0 is EOL too, but it works here. BTW, nothing should be broken in the older versions code wise / test wise - just installing the deps is ...painful.

@ukd1
Copy link
Author

ukd1 commented Aug 8, 2024

Also, feel free to checkout the build / what it's up to here - https://github.com/ukd1/canonical-rails/actions/runs/10304669552

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