Skip to content

FIX: Error when generating a controller. #78

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

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

dixpac
Copy link
Contributor

@dixpac dixpac commented Sep 30, 2021

This PR fixes error on the controller generation:

bin/rails g controller Home index

Since, I'd to override rails scaffold controller to work with rails 6, I
needed to configure rails controller generator also.

@dixpac
Copy link
Contributor Author

dixpac commented Sep 30, 2021

Fixes the main issue of #77 . Other issues noted there, we will fix later since they are less pressing atm :)

This PR fixes error on the controller generation:

`bin/rails g controller Home index`

Since, I'd to override rails scaffold controller to work with rails 6, I
needed to configure rails controller generator also.
@dixpac dixpac force-pushed the dix/fix_controller_generators_bug branch from 974d024 to f7cfd37 Compare September 30, 2021 21:55
@dhh dhh merged commit debe59c into rails:main Oct 1, 2021
@dhh
Copy link
Member

dhh commented Oct 11, 2021

Can you fix up #77?

@dixpac
Copy link
Contributor Author

dixpac commented Oct 11, 2021

I will try, resource generator I can, view-component not yet sure how :)

Main issue is that I've "monkey-patched" default rails generators in order to have the partial for rails 6, and that opened panadora's box and I'm not sure how much can we go with this monkey patch in relation with other engines such as view_component.

One thing I'm thinking we could do, but not sure if you are up for that is to have the partial on rails 7+ and no partial for rails 6 (UI design will stay the same)?

//index.html.erb
<% if Rails.version < 7 %>
  // loop with with the divs
<% end %>

@dhh
Copy link
Member

dhh commented Oct 12, 2021

Yeah, sounds like a better path to split them.

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