rubyatscale / packs-rails

packs-rails establishes and implements a set of conventions for splitting up large monoliths.
MIT License
268 stars 26 forks source link

Skip gems in RSpec integration #56

Closed alexevanczuk closed 1 year ago

iMacTia commented 1 year ago

This change just surprised me. We're in the process of isolating some of the application code into "gems", but right now the coupling between these and the rest of the codebase is pretty high, and we need to reduce it before we can properly isolate the code into a standalone gem.

Until then, the gems are effectively part of the monolith, and as such we'd like to run tests for it. Before this change, moving files into gems and their tests would have no major repercussions as tests would be run anyway.

But after this change, how are we supposed to correctly run tests for our gems?

alexevanczuk commented 1 year ago

@iMacTia We chatted a bit about this one and I'm wondering if you really need to have a .gemspec for these systems. I don't know if I'd call something a gem if they aren't published/distributed, aren't tested in isolation, and only run in the context of the monolith, even if it has a gemspec.

Is there some gem-related feature you're getting right now that you rely on? If not, have you considered just moving that code into a pack until it's ready to be a fully-fledged gem that has its tests run in isolation?

iMacTia commented 1 year ago

Thank you for the quick reply @alexevanczuk. Indeed I considered that as well and I can definitely relate to your conclusions.

The main reason I opted for making them gems is that we can have more control over dependencies. By adding a gemspec, I can now move specific dependencies from the root Gemfile and add them there. This will make it easier to move them out and publish them later on.

I think the bulk of the issue here is that this is a temporary step: these "gems" are not isolated enough to be moved out or tested in isolation, but we want to start having them as packages so we can better track ownership and packwerk violations, to help us get to the point where we can deploy them as real isolated gems.

I would also like to flip your question, which is what I asked myself before taking this decision: "what is the advantage of having fully-fledged gems inside the codebase, if they could already be deployed as external gems, and declare them as packs"? The only explanation I could think of is in the context of a monorepository (multiple projects living inside the same repo, but tested and deployed separately), but that's not something we want to do and potentially up to teams to decide if they want to leverage or not.

One possible solution would be to keep the current behaviour as the default, but leave the possibility to opt-in to make gems more integrated (i.e. skip the is_gem? check) to help use-cases different from a monorepository. Would you be open to a contribution in this direction?

alexevanczuk commented 1 year ago

we can have more control over dependencies ... I can now move specific dependencies from the root Gemfile and add them there.

I'm not following this... if the Gem is not tested in isolation then it is not actually using the list of dependencies in the gemspec. That is – the gemspec is completely ignored from a dependency bundling perspective and only the root Gemfile is used. Therefore it's equivalent to just having a plain text list (e.g. a comment in a pack saying what gems it uses) since it's not enforced. Is there a use case for having this list otherwise? Or is it executed somehow in a manner I'm not understanding?

but we want to start having them as packages so we can better track ownership and packwerk violations

That's a good goal but why does having a gem make this easier? Ownership and packwerk violations can both be tracked when the system is just a pack.

I would also like to flip your question

I can think of a bunch of reasons why we'd want to have a fully-fledged gem within a codebase (you do actually want to share them with other applications, having test isolation or other forms of development isolation, etc.). However, I'm still not understanding the advantage here of having a subsystem have a gemspec if it's not actually being developed on / deployed as a gem.

One possible solution would be to keep the current behaviour as the default, but leave the possibility to opt-in to make gems more integrated ... Would you be open to a contribution in this direction?

The reason I'm hesitating here is because I still don't totally understand the use case so far. One of our goals with rubyatscale is to not step on the toes too much of the existing community solutions. That is, we've been thinking of a pack as a subsystem that may be still entangled with other systems in various ways, but a gem is a pretty strictly defined thing – we don't want to encourage too much expansion/dilution of what being a gem is. Or to put it another way – even though packs are okay never becoming a gem, a pack can also be an aspirational gem and we hope the toolchain can provide mechanisms to help make the pack => gem transition smoother, and we're more likely to encourage contributions to that end. For example, I think it would be really interesting to experiment with a packwerk checker to enforce_gem_dependencies that functions like enforce_dependencies but has a key gem_dependencies. Maybe with such a thing, it would even be possible to dynamically run tests for a pack with a "virtual Gemfile" of only the gem dependencies (transitively). I don't know if such a feature is useful/practical in practice, but mostly just trying to provide an example of how we can support packs becoming gems (rather than gems becoming more like packs).

iMacTia commented 1 year ago

Thanks @alexevanczuk, I completely get what you're saying and I realise what we're trying to do is not very "purist" 😅. I guess we're trying to make this packages into some sort of thing-in-between a gem and a package. But you're right this does not have tangible benefits and we could find other workarounds.

I guess the choice is on our side to either revert them back to simple packages (and note gem dependencies somewhere else, as you suggested) or to go all-in with the gem idea, meaning they should be bundled and tested in isolation to ensure the gemspec is correct.

I'll discuss this one with the team and we'll decide how to proceed, thank you for taking the time to explain your reasoning

alexevanczuk commented 1 year ago

Awesome, thanks for your understanding and the thoughtful dialogue. Let me know if I can help you or the team chat through anything 🙏