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

Ensure necessary dependencies are included in bullet_train.gemspec #761

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

gazayas
Copy link
Contributor

@gazayas gazayas commented Jan 31, 2024

Closes #714.

I also added bullet_train-super_scaffolding because we have the following lines in the Membership controller base:

has_many :scaffolding_completely_concrete_tangible_things_assignments, class_name: "Scaffolding::CompletelyConcrete::TangibleThings::Assignment", dependent: :destroy
has_many :scaffolding_completely_concrete_tangible_things, through: :scaffolding_completely_concrete_tangible_things_assignments, source: :tangible_thing

I wanted to added bullet_train-api because of this line:

However, bullet_train-api already depends on bullet_train:

https://github.com/bullet-train-co/bullet_train-core/blob/main/bullet_train-api/bullet_train-api.gemspec#L37

So, trying to include bullet_train-api was giving me this error:

> bundle install
Your bundle requires gems that depend on each other, creating an infinite loop. Please remove either gem 'bullet_train' or gem 'bullet_train-api' and try again

Gemfile and bullet_train.gemspec

We're including a lot of in-house gems in the Gemfile and not the gemspec. In some instances, we're including BOTH:

gem "bullet_train-api", path: "../bullet_train-api"
gem "bullet_train-fields", path: "../bullet_train-fields"
gem "bullet_train-roles", path: "../bullet_train-roles"
gem "bullet_train-super_scaffolding", path: "../bullet_train-super_scaffolding"
gem "bullet_train-super_load_and_authorize_resource", path: "../bullet_train-super_load_and_authorize_resource"
gem "bullet_train-themes-light", path: "../bullet_train-themes-light"
gem "bullet_train-has_uuid", path: "../bullet_train-has_uuid"
gem "bullet_train-scope_validator", path: "../bullet_train-scope_validator"
gem "bullet_train-themes", path: "../bullet_train-themes"

I'm curious if we could learn from the rails repository. It looks like they don't have Gemfiles for their repositories but include everything only in their gemspecs (which I'm under the impression that's what we SHOULD do). Would be glad to undertake that task if that's the direction we should head in.

Active Record

https://github.com/rails/rails/blob/774c630068e69547fabd6bddedd62a712ae7e736/activerecord/activerecord.gemspec#L38-L39

Action View

https://github.com/rails/rails/blob/774c630068e69547fabd6bddedd62a712ae7e736/actionview/actionview.gemspec#L36-L44

@gazayas
Copy link
Contributor Author

gazayas commented Jan 31, 2024

This one is a little more challenging than expected. With the current diff, you can see that bullet_train-fields already includes its own version of bullet_train-super_scaffolding, so I deleted bullet_train-super_scaffolding from the Gemfile and added bullet_train-fields in the gemspec. This is throwing errors for other gems though:

The gemspecs for path gems changed, but the lockfile can't be updated because
frozen mode is set

We technically DO include bullet_train-fields in the Gemfile:

gem "bullet_train-fields", path: "../bullet_train-fields"

It also exists within the bullet_train environment:

> pwd
/home/gazayas/work/bt/bullet_train/local/bullet_train-core/bullet_train
> rails c
Loading development environment (Rails 7.0.3.1)
irb(main):001> BulletTrain::Fields
=> BulletTrain::Fields

I feel like this is an issue that goes beyond the scope of #714, and perhaps we should re-frame it as how we include our dependencies in general.

@jagthedrummer
Copy link
Contributor

I feel like this is an issue that goes beyond the scope of #714, and perhaps we should re-frame it as how we include our dependencies in general.

Yeah, I think this is right. I think all of the BT dependencies are somewhat arbitrarily declared. Everything mostly works itself out by the time you have everything added in the Gemfile of the starter repo, but our declared dependencies don't seem to accurately match our expected/actual dependencies.

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.

Add explicit dependency from bullet_train => bullet_train-fields
2 participants