Relax activesupport dependency

[alessandro-fazzi]

Given #226 and the bleeding announced in https://github.com/adomokos/light-service/issues/153#issuecomment-528679739 the goal of this PR is to relax the dependency this gem has on ActiveSupport.

Why not removing it completely?

It turns that the only part of the gem which continues to require ActiveSupport are the Rails generators. They were introduced recently in https://github.com/adomokos/light-service/pull/194. Thus I cannot afford to remove ActiveSupport with a one-shot PR. Nor I want to take decisions about how to continue from now on w/o including maintainer and community into the discussion.

Is it worth to add more dependencies w/o removing ActiveSupport?

Ideally not, but from a transitional perspective, yes. Now we have a single well scoped feature requiring ActiveSupport instead of having code bites spread all over the code base. I'm bringing the gem in a more free and easier to manage position in regard of AS dep.

What about the two new dependencies?

I added https://dry-rb.org/gems/dry-inflector/ and https://github.com/schmidt/structured_warnings

Both were choosen because they are small, dependency-free gems with single functionality.

dry-inflector is from the (beloved by me) dry-rb ecosystem (https://dry-rb.org/). Neat, iper-atomic and well written. It is the Hanami inflector.

structured_warnings is a neat, very small gem; it has not been updated recently, but IMO it doesn't need to be. It works on ruby 3.1 and its approach is simple and far (FAR) less polluting than ActiveSupport's one. I was able to implement a custom matcher for rspec too, writing expressive tests. Moreover its interface made possible to do a surgical substitution in the actual code base.

---

From my point of view, being the maintainer of the project, I'd think about splitting rails generators into a separate gem. This would enclose ActiveSupport and Rails-only things in cleaner context, easier to maintain. And the test suite of the "core" gem would lose all the complexity inherent to testing against all the possible AS versions.

I thought to open this one as a draft, but I decided to concretely propose something :) No problem turning it into draft or to use this just as a discussion desk and then close it.

Thanks for your attention.

[adomokos]

This is a great PR! I thank you for the work you put into it, I am going to merge it. If I understand this correctly, after your change, the only dependency we have for AS is the Rails generator.

You wrote:

From my point of view, being the maintainer of the project, I'd think about splitting rails generators into a separate gem

I am all for doing this, but we should move LightService under an org, and have the gem, and this other one with Rails generator should be there. What do you think?

[adomokos]

@gee-forr - what do you think?

[alessandro-fazzi]

This is a great PR! I thank you for the work you put into it, I am going to merge it. If I understand this correctly, after your change, the only dependency we have for AS is the Rails generator.

You wrote:

From my point of view, being the maintainer of the project, I'd think about splitting rails generators into a separate gem

I am all for doing this, but we should move LightService under an org, and have the gem, and this other one with Rails generator should be there. What do you think?

Reading again the code, I’d like to do a couple of little refinements. But I’ll let the discussion go further before to do anything else.

I think, consolidated the choice to split the code, the organization would be the more elegant solution and the better guarantee for the community. But pragmatically speaking it wouldn’t be mandatory.

I don’t think an organization would be that big management complication, but it’s a personal point of view and a matter to think about before going all-in.

[gee-forr]

@adomokos - I really appreciate you asking for my opinion ❤️

@pioneerskies - Thank you so much for the PR and the interest.

I haven't looked at the actual changes yet, but here are my opinions, off the cuff.

• We cannot remove rails generators without marking them for deprecation, and then removing them in a major release. So, if we release a 0.19.0 and they output deprecation warnings whenever rails generators are used, that's perfect.
• Moving LS to an org would be nice - it'll also make discovering gems in the LS ecosystem easier. I suspect @douglasgreyling - author of light-service.js and light-service.php would be keen to move his projects over to the org too.
• Moving rails generators to a light-service-rails gem would be great - assuming deprecation is handled responsibly, and would make maintaining the two gems simpler. I would want to match versions between them so that there's no confusion between which version of light service the rails gem bundles in. As it is, there are some bugs/quirks with the generators that I'm not happy with that I've been meaning to fix, but don't really want to impose a version bump on LS as a result.

[gee-forr]

@adomokos - if you're happy to move forward with this, I am. I think adding two more dependencies is a small and temporary price to pay for what it unlocks for the future.

If you want to move forward with the org + separate rails gem, then, @pioneerskies could I ask that you also add deprecation warnings to the rails generators?

[alessandro-fazzi]

If you want to move forward with the org + separate rails gem, then, @pioneerskies could I ask that you also add deprecation warnings to the rails generators?

I'd be happy to do, once decisions will be made, but I'd opt to do it in a dedicated PR: current PR opens the question, but it doesn't want to directly move away generators. A new PR using this one as backbone would be clearer and tidier IMO.

[adomokos]

I am afk, but will pick this up, thoroughly review it, and merge it next week. Thanks for your patience there!

[alessandro-fazzi]

I am afk, but will pick this up, thoroughly review it, and merge it next week. Thanks for your patience there!

I'll be on vacation starting from 1st until 19th July. In the mean time feel free to do what it's needed with "my" code. I've already annoted and subscribed desirable updates and expressed all the opinions I had to. So feel free to act w/o my intervention in the coming weeks :)

Thank you all so much.

[adomokos]

I like this, will merge this. It's not fixing a critical bug. We'll get this in.

[adomokos]

Hello @pioneerskies! Sorry for keeping this PR open for so long.

I thought about this while I was away. How hard would it be not to introduce two (albeit small) gems to light-service, but leverage pure Ruby to accomplish this functionality? I'd love to have LS be outside gem-dependent-free if possible in the future.

[alessandro-fazzi]

Hello @pioneerskies! Sorry for keeping this PR open for so long.

I thought about this while I was away. How hard would it be not to introduce two (albeit small) gems to light-service, but leverage pure Ruby to accomplish this functionality? I'd love to have LS be outside gem-dependent-free if possible in the future.

Hi there,

AFAIK deprecations could be simplified down to a custom Logger: if my assumption would be right, then writing 1st party code would be easy. I mean "easy" as "not complex", not as "fast", but that's it.

The inflector instead would be different: dry-inflector is the smallest, most low-complexity thing I've found around. Given we don't want to use any 3rd party dep, then I'd opt for brutally copy/paste-ing the gem's lib/ into our codebase. That's because I won't be able to design something different and more simple than that code and reinventing the wheel would be wasted time with a worse final result (since I'm honestly light years far from Luca's and Peter's design/coding skills :) )

Conclusion: a custom "deprecation logger" + a savvy copy-paste should result in a quite small and not complex goal to achieve.

[adomokos]

I see adding 3 ruby gems with this PR:

1. i18n - which needs concurrent-ruby
2. dry-inflector - no external library dependency
3. structured_warnings - no external library dependency

I do like the intent of this PR, but I am now hesitant to merge it. We are adding quite a bit of dependency, and I don't see the path of cutting out activesupport altogether. I wonder if we should keep this PR open, create a new gem light-service-rails, extract the Rails generator dependencies into that, and then merge this PR. That way we have a guaranteed path of reducing the external gem dependency.

What do you think about that?

I am happy to create a new repo for light-service-rails, I am not sure if I'd create a new organization for it yet. I'd be happy to add you as maintainer to this new project if you're willing to help me extract the Rails and AS dependencies. What do you think?

[alessandro-fazzi]

I like to re-underline that i18n here is not added, but just made explicit: it's already an AS's dependency. As it stands it's just a peer dependence through AS but it is explicitly used for failure message translation. Having it as a direct dependence was a way to underline the fact that even in the will to remove AS we'll have to retain i18n.

IMO we should be able to avoid i18n dependency too, making it opt-in: if an implementor would like to use translated messages she should add i18n to her bundle and LS would pick it up automatically. If absent and a Symbol is passed as failure message, LS could drop a warning ad just .to_s the symbol.

I know this i18n-related discourse may seem OT, but I consider part of the "dependent-free" path you are picturing.

Coming on the spot of your comment now :)

I do like the intent of this PR, but I am now hesitant to merge it

I'm not only okay with that, but I'm really happy with that. My intention was to open a discussion bringing a practical substrate to it.

We are adding quite a bit of dependency

I think we're trading dependencies here, more than adding. The biggest fault of this PR, in my opinion, is that it moves from one dep from another w/o layering them with first party wrappers. Wrappers would be a refactoring pattern making possible to plug in/out external deps or first party code w/o touching other parts of code. E.g.:

# lib/light-service/inflector.rb

require "dry/inflector"

module LightService
  class Inflector
    attr_reader :inflector

    def initialize
      @inflector = Dry::Inflector.new
    end

    def unerscore(string)
      inflector.underscore(string)
    end

    def pluralize(string)
      inflector.underscore(string)
    end
  end
end

# lib/light-service/localization_adapter.rb

module LightService
  class LocalizationAdapter
    attr_reader :inflector

    def initialize
      @inflector = LightService::Inflector
    end

    # [...]

    def i18n_scope_from_class(action_class, type)
      "#{inflector.underscore(action_class.name)}.light_service.#{inflector.pluralize(type.to_s)}"
    end
  end
end

This way we'd be able to swap in/out different external deps or get rid of them and write our own or anything we'd like to, with a single point of responsibility

I wonder if we should keep this PR open, create a new gem light-service-rails, extract the Rails generator dependencies into that, and then merge this PR

That path would be for sure the more profitable one.

Current PR had just the goal to make things move forward given the actual code base and to poll the interest around the proposed mid-term goal.

Keep this one open as draft and we'll understand if it will be better to update it or to close it for a new one.

I am happy to create a new repo for light-service-rails, I am not sure if I'd create a new organization for it yet. I'd be happy to add you as maintainer to this new project if you're willing to help me extract the Rails and AS dependencies. What do you think?

• don't bother to go for the org atm: let's do some work and then you'll think about organizational matters
• I'll be happy to help until the end of September; then my baby-girl will come to the world and I'm ready to bet I'll need to take a pause from free-time work for a while :)
• let's check for @gee-forr availability to help too; even just as reviewer. As original author of generator's code and as active user of the feature he may be a great booster

[adomokos]

If and when we go with the route of a separate gem version for light-service-rails, we will need to bump the major version to 1.

[adomokos]

Closing this, as AS was removed with this PR.