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

Namespacing and isolating engine. Include documentation to mount and … #60

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

adriancb
Copy link

An issue we discovered while integrating Abraham that could be rather common is that routes from integrated engines are appended to the app routes.

We have a catch-all route as the final directive that means routes appended via Rails.application.routes.draw become unreachable. In this instance the /abraham_histories route was unreachable.

This PR attempts to isolate and namespace Abraham so that it can be mounted separately.

Given the additional mounting requirement, the suggestion is that a major version is released.

…run tests. Using named route in helper partial. Including helper on init. Upgrading Ruby from 2.5.3 > 2.7.3. Upgrading Rubocop and linting accordingly.
@adriancb adriancb force-pushed the feature/namespace_and_isolate branch from 636c2f9 to 4ee033e Compare June 29, 2021 01:04
@jabbett
Copy link
Contributor

jabbett commented Jun 29, 2021

@adriancb Thanks so much! This seems like a responsible thing to do, and all the tests pass :) I'll review with our team.

A couple questions:

  • Since this is the only Rails engine I've developed, I'm curious to know how other engines handle this. Do you have a handy example to share?
  • You've upgraded the Ruby version — does this change require it, or is it only incidental? I ask because it suggests to me that our continuous integration may want to test not only multiple Rails versions but also multiple Ruby versions.

@adriancb
Copy link
Author

👋 @jabbett; thanks for the feedback - great!

In terms of examples, a couple of examples (of libraries we use) spring to mind:

I believe that at its core, an engine is just a Rack app, it could be Sinatra as per below:

Hope that helps! 😄

In terms of the upgraded Ruby version; this change does not require 2.7.x and neither does Abraham, but thought it was good practice to bump to the latest minor version. I'm unsure you need to support multiple Ruby versions. Happy to yank the change if it causes confusion/etc.

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