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

reduce rails dependencies #7

Merged
merged 1 commit into from
Oct 17, 2022

Conversation

BenMorganIO
Copy link
Contributor

@BenMorganIO BenMorganIO commented Oct 7, 2022

Hey there,

I noticed that your ruby gem was including all of rails. I've trimmed it down quite a bit to just the dependencies that this gem really needs.

A problem I encountered was that the test coverage didn't seem very good and the build was failing upon install. Please take a look at this problem in the future.

For the more optional dependencies, I would add them as either optional or as development dependencies.

@gyfis
Copy link
Contributor

gyfis commented Oct 9, 2022

@BenMorganIO Hey Ben! Thanks for the PR. Can you share your use case where you use this gem outside of rails, please? We also have a lower-level rack-based gem, would that fit your use case better?

@BenMorganIO
Copy link
Contributor Author

It's fairly standard. My application will not need to use action cable or action text. So I instead opt for the individual rails deps. This way I don't have to pull in as many dependencies into my application.

@adikus
Copy link
Contributor

adikus commented Oct 10, 2022

Hi @BenMorganIO Thanks! This looks good - ignore the failing tests, looks like we have some setup issues in there for a couple of ruby versions.

I'd be happy to merge this, but I would like to understands how this affects your particular use case.

Would you be able to provide an example Gemfile and an application config with which I would be able to reproduce this gem requiring more than the original app was?

@BenMorganIO
Copy link
Contributor Author

BenMorganIO commented Oct 10, 2022

Here's an example section of a Gemfile where we replace the gem 'rails', '~> 7.0.4' with instead selecting which gems we want:

# Not using: actioncable actionmailbox actiontext
RAILS_DEPENDENCIES = %w[
  activesupport actionpack actionview activemodel activerecord actionmailer
  activejob activestorage
].freeze

RAILS_DEPENDENCIES.each { |dep| gem dep, '~> 7.0.4' }

Now that there's no action cable, action mailbox, or action text. My application is smaller, should in theory startup quicker, and I will have less of a time in dependency hell doing updates.

If my Rails app were API only and I wasn't sending any emails out or storing anything (perhaps just adding an API layer over the database), I could in theory reduce it even further by yanking active storage and action mailer, which would in turn remove a large amount of dependencies that they carry.

This also helps with security by only pulling in the dependencies that you need.

@adikus adikus merged commit d7b3429 into logtail:main Oct 17, 2022
@adikus
Copy link
Contributor

adikus commented Oct 17, 2022

Thanks @BenMorganIO, sorry about taking so long to get this reviewed and merged!
I've released this as version 0.1.7

@BenMorganIO BenMorganIO deleted the reduce-rails-dependencies branch October 17, 2022 15:29
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.

3 participants