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

Migrations #5

Closed
bf4 opened this issue Jul 31, 2013 · 12 comments
Closed

Migrations #5

bf4 opened this issue Jul 31, 2013 · 12 comments

Comments

@bf4
Copy link

bf4 commented Jul 31, 2013

I see you require running rake gutentag:install:migrations (and I gather that this works without a configuration in the engine because you used a sortable migration name https://github.com/pat/gutentag/blob/e9af72b/db/migrate/1_gutentag_tables.rb )

Did you consider including the migrations without need of a rake task?

# create config your gem root
# in config/application.rb
config.paths["db/migrate"] += Gutentag::Engine.paths["db/migrate"].existent
@pat
Copy link
Owner

pat commented Aug 1, 2013

I'd not considered this as an option... but it's made me think about migrations in engines. Off the top of my head, there are three options:

  1. Migrations in db/migrate without the rake task (as you've pointed out).
  2. Migrations in db/migrate with the rake task (as Gutentag currently uses).
  3. Migrations created by a generator.

And then, here's what I think matters from both Rails app developer and engine developer perspectives:

  • An app's migrations end up in one location.
  • An app's migrations have a single clear numbering system.
  • An engine's migrations are not edited over time, but further migrations are instead added - much like an app.

Option 1 takes care of the third point, but it splits migrations over more than one location, and also introduces separate numbering systems (I don't want my migration version numbers reflecting when an engine developer added a migration, but when I added that update to my app).

Option 3 takes care of the first and second points, but it then gets messy from a maintenance perspective - unless the engine developer adds new migration generators instead of modifying the existing generator templates.

Option 2 covers off all three points, hence why I'm happy to stick with it.

Of course, I only thought through all of this today. I'm sure there's perspectives and advantages I've not considered.

@sporto
Copy link

sporto commented Aug 1, 2013

rake engine:install:migrations is the cleanest option as it changes the date stamp of the migration to the moment you ran the rake task. The order of migrations in your app could be important.

@bf4
Copy link
Author

bf4 commented Aug 1, 2013

Good points, I hadn't considered them all. I had only thought about someone updating the engine code and forgetting to run the migration. We really need to work on the engine docs. Look what I ended up having to do here: https://github.com/mbleigh/acts-as-taggable-on/pull/343/files#L2R47 :)

@pat
Copy link
Owner

pat commented Aug 1, 2013

Yeah, that's not cool at all. That's the main reason why I'm not a fan of migrations through generators.

@bf4
Copy link
Author

bf4 commented Aug 1, 2013

@pat You mean that you'd rather dump 'em all in "db/migrate/1_gutentag_tables.rb"? It all comes down to the same thing in Rails engines, though. It sorts them and copies them.

@parndt
Copy link

parndt commented Aug 1, 2013

In my opinion, option 2 rake gutentag:install:migrations is the cleanest and best option available. It gives the main application full control over the order that its database is migrated and once you've copied one in it will skip it thereafter. This has the caveat that an engine developer has to treat migrations as a backward-compatibility API and not edit them later on except for perhaps during major version increment a la Semantic Versioning.

Before rake railties:install:migrations existed (back when engines were experimental and I was trying to proliferate their use), I had to write complex code to manually handle copying or running the migrations (like mbleigh/acts-as-taggable-on#343) and I'm relieved that is no longer the case. The great news is that Rails Engines should no longer have to do anything at all to benefit from Rails' APIs here except follow naming conventions, which @pat is doing.

@pat
Copy link
Owner

pat commented Aug 2, 2013

Thanks all for the feedback - I'm going to close this ticket. The opportunity to think through this and get your opinions is appreciated :) If anyone else wishes to chime in, they can certainly do so.

@pat pat closed this as completed Aug 2, 2013
@radar
Copy link

radar commented Aug 2, 2013

I agree with @pat and @parndt on this issue (although probably a bit late!). We have a consistent problem with Spree where people will upgrade their Spree version and then forget to run the migrations! The second option is still the best way to solve this problem, as far as I know.

You could go one step further and provide a binary that does the copying+running of the migrations, something like gutentag migrate, but that'd just be laziness. :)

@bf4
Copy link
Author

bf4 commented Aug 2, 2013

I'd be interested to hear Jose Valim's take on this since he's the one who introduced me to 'option 1', and his book is all about the engines and their craftiness.

There @josevalim wrote:

Copying with the rake task is useful when you want to explicitly control the migrations you copy from an engine. The configuration above ['option 1' -BF] is useful when you trust the engine and want all migrations in the engine to run automatically.
I would use a generator whenever the migrations are dynamic. For example, Devise does that since you can have a migration for a User model, an Admin model, whatever you come up with. In this plugin's case though, the migration is "static", so I would just copy using the rake tasks or the configuration above.

@benjaminleesmith also advocated 'option 1', in the context of modularizing an existing app

cc: @zzak , @senny , @fx, @steveklabnik , @Dracco

Anyone want to work on the engines guide in docrails with me? Disclaimer: haven't started yet.

@josevalim
Copy link

@bf4 I think "trust" was a poor choice of word in there. Ownership is a better word.

I would use the first approach just if you "own" the engine, not for 3rd parties one. So if you are splitting your applications into multiple engines, the 1st approach is a good option. The 2nd approach should definitely be the recommended one for the majority of the cases.

I will amend the comment. :)

@bf4
Copy link
Author

bf4 commented Aug 8, 2013

There's an alternative configuration we forgot:

Rather than depend on the rake task, the end user can add his/her/zer/their codebase in config/application.rb or in an initializer, if the end user prefers to automatically run migrations.

config.paths["db/migrate"] += Gutentag::Engine.paths["db/migrate"].existent

@pat
Copy link
Owner

pat commented Aug 9, 2013

Sure, but then the migration version numbers are out of the developer's control - it's the equivalent of the first option in my list.

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

No branches or pull requests

6 participants