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

[Backport] Test on Rails 8 #3358

Merged
merged 4 commits into from
Dec 2, 2024
Merged

[Backport] Test on Rails 8 #3358

merged 4 commits into from
Dec 2, 2024

Conversation

jcoyne
Copy link
Member

@jcoyne jcoyne commented Oct 14, 2024

No description provided.

@jcoyne jcoyne added this to the 8.x milestone Oct 14, 2024
@jrochkind
Copy link
Member

I'm not sure main is actually currently passing on Rails 8? Not sure what happened.

@jrochkind
Copy link
Member

Love the backport of Rails 8 to Blacklight 8.x, thanks!

@jrochkind
Copy link
Member

jrochkind commented Nov 26, 2024

I wanted to see if I could solve this one.

But I can't seem to get straight release-8.x branch to pass CI locally. Or this branch to run CI locally without many more failures than are displayed in CI.

(I can get main CI to pass locally, at least except for one apparently flakey JS test).

Not sure what I'm doing wrong, curious if anyone else can run CI locally for BL 8? Without being able to run locally, I'm having trouble figuring out how to debug and identify the problem in CI.

I would love to have a BL 8.x release that supports Rails 8, if feasible!

@jrochkind
Copy link
Member

jrochkind commented Nov 26, 2024

OK somehow not sure how got it to run locally. Think I found the problem.

resource :catalog, only: [:index]

Generated into app, is illegal becuase a singular resource doesn't have :index as a valid action.

In main, it looks like this generates in as resource :catalog, only: [] instead. (then has other routes added by concerns).

Just making this change to generated .internal_test_app doesn't give me green tests though... but I've had so much trouble with them I"m not sure if that's a local problem or not. Will look for commit in main that made this change to backport.

This is probably also a good upgrade note in BL9 for anyone upgrading from previous BL, who wants to go to Rails 8 -- as well as a note for BL8 users that want to go to Rails 8, to be put somewhere!

@jrochkind
Copy link
Member

74008cf

@jcoyne jcoyne merged commit d69ccde into release-8.x Dec 2, 2024
14 checks passed
@jcoyne jcoyne deleted the backport-test-8 branch December 2, 2024 21:15
@jrochkind
Copy link
Member

jrochkind commented Dec 2, 2024

Sweet, thank you!

I wonder if there's a place we can or should put release notes for anyone upgrading an existing BL app to Rails 8 -- whether BL 7 or BL 8, the warning is:

If you have in your config/routes.rb something like that looks like:

resource :catalog, only: [:index], as: 'catalog', path: '/catalog', controller: 'catalog'

Which was generated by a past version of Blacklight. When upgrading to Rails 8, change it instead to:

resource :catalog, only: [], as: 'catalog', path: '/catalog', controller: 'catalog'

(The value for only should be empty array instead of [:index].)

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