Open adriancb opened 3 years ago
@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:
👋 @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.
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.