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

Simplify dependency managment in engine installs #227

Open
jensljungblad opened this issue Feb 21, 2017 · 11 comments
Open

Simplify dependency managment in engine installs #227

jensljungblad opened this issue Feb 21, 2017 · 11 comments

Comments

@jensljungblad
Copy link
Collaborator

jensljungblad commented Feb 21, 2017

Can we simplify Gemfile vs .gemspec when installing in an engine? It should at least be possible to automatically require all things specified in the .gemspec file. People not familiar with creating gems and engines have been confused about the fact that you need to manually require dependencies when specifying them in a .gemspec. Not sure this is something we should solve though.

Here's an example of requiring all runtime dependencies:

Gem.loaded_specs["admin"].runtime_dependencies.each do |dep|
  begin
    require dep.name
  rescue LoadError
    require dep.name.gsub("-", "/")
  end
end

require "admin/engine"

module Admin
 # Your code goes here...
end

However, this does not handle gems such as godmin-tags that needs to require a file called godmin/tags. How does bundler handle this?

@jensljungblad jensljungblad created this issue from a note in Godmin 2.0 (Backlog) Feb 21, 2017
@jensljungblad
Copy link
Collaborator Author

jensljungblad commented Feb 23, 2017

This is how bundler does it: rubygems/bundler@a0d97c4

It's possible Bundler.require(:default) would do the trick. However, the bundler people recommend against having Bundler as a runtime dependency of gems, which engines are. I guess in case you ever want to distribute them on Rubygems. Not sure that is something that would ever happen here though.

@jensljungblad
Copy link
Collaborator Author

jensljungblad commented Feb 23, 2017

This works, but not sure if it should be part of this gem or if this is something people could add if they don't like manually requiring gems in their engines...

Gem.loaded_specs["admin"].runtime_dependencies.each do |dep|
  begin
    require dep.name
  rescue LoadError
    require dep.name.gsub("-", "/")
  end
end

Perhaps with a comment that explains what it does and what it can be replaced with?

@Linuus
Copy link
Contributor

Linuus commented Feb 24, 2017

Hmm I don't think we should add it by default... At least not until we've tried it a bit more ourselves first :D Perhaps it can just be documented in the readme or something for now?

Making it a manual process to require gems would also be a good fit for our "less magic" philosophy. We should probably document that the user must require gems manually in the readme as well. Even if it's not godmin specific.

@jensljungblad
Copy link
Collaborator Author

Yeah I guess the difference is that its a Rails convention to automatically require gems. People who just build Rails apps are not familiar with other setups. Rails kinda hides the fact that they use Bundler.require in a config. If you're a Ruby programmer first you know all about requiring dependencies, but if you've only been doing Rails, not so much. And when we suggest people use engines we get this issue. So I'm a bit torn.

If we add it I think we should do it with a generator though, so it's easy to remove. Not hide it somewhere in the gem.

@Linuus
Copy link
Contributor

Linuus commented Feb 24, 2017

Sure, but everyone who uses engines already will be confused that they don't have to require? :) That's probably less people though. Maybe we should nudge people in the right direction and move away from auto require.

Are there still gems that don't follow this naming convention? If so, it would be confusing to have to require those manually for no apparent reason. But perhaps there are no such gems anymore :)

@jensljungblad
Copy link
Collaborator Author

jensljungblad commented Feb 24, 2017

Anyone who knows they'll have to require dependencies will open that file in order to require the dependency. They will then see that piece of code and a comment explaining what it does, and can choose between keeping it or removing it in favour of manually requiring. I guess? Thing is we can't really move away from auto require, since we can't change the fact it's on by default in Rails, for the main app. So if you have a standalone Godmin without any engines, you get auto require.

You mean the dash naming convention? No it's not followed by everyone which is why Bundler does that thing I linked to above, and which is part of the snippet I posted above as well.

@Linuus
Copy link
Contributor

Linuus commented Feb 24, 2017

I meant if some gems do something even weirder.

gem: foo-bar-baz
File to require is lib/foo/bar_baz.rb

That would not be caught by the require above. It would blow up in bundler as well, unless they have more code to handler even weirder stuff :D

Yes, I know it'll be a difference between engine installs and standalone. But other things are different as well. The .gemspec for instance. Why declare dependencies there and not in a Gemfile, like in the standalone case?

I just feel like this isn't an issue with godmin. It's the way engines work :)

@jensljungblad
Copy link
Collaborator Author

Yeah I think Bundler.require would explode with that example as well.

Well, Bundler.require is something that Rails adds, it's not something that's included for every Ruby application. So I guess Godmin could do something similar with engines. But I agree, I'm not sure about this one!

@jensljungblad
Copy link
Collaborator Author

jensljungblad commented Feb 13, 2018

Okay, here's another idea. Let's just not add dependencies to the gemspecs at all? We could stop promoting the practice of putting godmin in admin/admin.gemspec in favor or placing it in Gemfile. You get autoloading, and in projects with multiple admin engines, you don't have to specify the same dependencies over and over, having to keep track of the versions etc.

Keeping dependencies in an engine's gemspec seems useful only if you plan on extracting the engine to a gem. But if that is not the use case we are building for, maybe we should change that.

This would involve changing some generators, mainly, and the documentation.

@Linuus
Copy link
Contributor

Linuus commented Feb 13, 2018

Yep. Perhaps that is a better idea.

Any other drawbacks we can think of other than “gemification” of an engine?

@jensljungblad
Copy link
Collaborator Author

Well, I had some memory that one reason we did this was so that engine binstubs, admin/bin/rails, would find the godmin generators. I tested this briefly yesterday and there was no problem, but we should probably test this on a project first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Godmin 2.0
Discussions
Development

No branches or pull requests

2 participants