This project is archived and is in readonly mode.

#3820 ✓resolved
Szymon Nowak

Make Rails::Generators::Migration#next_migration_number method a class method, so it possible to use it in custom generators

Reported by Szymon Nowak | January 30th, 2010 @ 05:27 PM

Currently when writing a migration generator that uses Rails::Generators::Migration#migration_template method, one needs to copy the code for "next_migration_number" method i.e. from ActiveRecord::Generators::Base.

This patch changes "next_migration_number" method into a class method, so developers can access it from their own generators:

def self.next_migration_number
  ActiveRecord::Generators::Base.next_migration_number
end

However, this would require developers to write the same piece of code in every migration generator, so I changed the default Rails:Generators::Migration#next_migration_number to automatically call the proper implementation based on the selected ORM library. The code is a bit ugly and it should check for LoadError and if the ORM::Generators::Base actually implements the "next_migration_number" method, but it's just a suggestion. This would allow to write a generator like this one: https://gist.github.com/5d0b8f8e510a9abcefaa.

Comments and changes to this ticket

  • Repository

    Repository February 2nd, 2010 @ 10:33 AM

    (from [17bee0dd2fcce2d040bd6edda3e19cb11c5813d9]) Change Rails::Generators::Migration protected instance methods to class methods. [#3820 status:resolved]

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/17bee0dd2fcce2d040bd6edda3e19c...

  • José Valim

    José Valim February 2nd, 2010 @ 10:40 AM

    • State changed from “new” to “resolved”
  • Steve

    Steve April 23rd, 2010 @ 07:09 AM

    It looks like the changes in #next_migration_number for this were reverted in an immediately following commit on the same day(101a8fa5f8cbf0f981ca984a279fb9838c79a751). Was that intended?

  • kmandrup

    kmandrup May 18th, 2010 @ 03:43 PM

    Please do the fix again :)
    I have been banging my head against this issue all morning, until I found this ticket!
    Hope it will be fixed in the next Rails (beta4?) release

  • Lailson Bandeira

    Lailson Bandeira June 21st, 2010 @ 04:03 AM

    Why the fix was reverted? Any problem with it?

  • David Jones

    David Jones August 10th, 2010 @ 11:51 PM

    • Importance changed from “” to “Low”

    When running my own generator that tries to use the migration_template method I get:

    /Users/djones/.rvm/gems/ruby-1.8.7-p299/gems/railties-3.0.0.rc/lib/rails/generators/migration.rb:30:in next_migration_number': NotImplementedError (NotImplementedError)

    Can someone change this ticket to open?

  • Oleg

    Oleg October 13th, 2010 @ 09:07 PM

    Wait. Why is this resolved?

    /Library/Ruby/Gems/1.8/gems/railties-3.0.0/lib/rails/generators/migration.rb:30:in next_migration_number': NotImplementedError (NotImplementedError)

  • José Valim

    José Valim October 13th, 2010 @ 09:22 PM

    It is a class method, but it is implemented by child classes (like AR::Generators::Base) and not Rails::Generators::Migration.

  • Oleg

    Oleg October 13th, 2010 @ 09:49 PM

    I guess the root question is what's the proper way to create generator with a migration? This is what I have:

    class GemGenerator < Rails::Generators::Base
      include Rails::Generators::Migration
    
      def generate_migration
        migration_template 'gem_migration.rb', 'db/migrate/gem_migration.rb'
      end
    end
    

    This blows up with the "next_migration_number: NotImplementedError" error. It's hard to piece together what's the proper way to go about it. I'm sure I'm not the only one.

  • José Valim

    José Valim October 13th, 2010 @ 09:55 PM

    If you are writing a migration for active record, you need to add this to your generator:

    def self.next_migration_number

    ActiveRecord::Generators::Base.next_migration_number
    

    end

    Try checking Rails::Generators::Migration source code, it may help.

  • Oleg

    Oleg October 13th, 2010 @ 10:28 PM

    I got it running, but with extra require and include it basically became this:

    def self.next_migration_number(dirname)
      orm = Rails.configuration.generators.options[:rails][:orm]
      require "rails/generators/#{orm}"
      "#{orm.to_s.camelize}::Generators::Base".constantize.next_migration_number(dirname)
    end
    

    Works now, but I still don't understand why it has to be like this.

  • José Valim

    José Valim October 14th, 2010 @ 06:54 AM

    Your migration template only supports Active Record, so why do you want to make it ORM agnostic? This is exactly why Rails should not have the code above. Because people would assume a migration template would work for all ORMs and it definitely won't.

    Just require 'rails/generators/active_record' at the top of the file and reference ActiveRecord::Generators::Base directly as I showed previously.

  • Oleg

    Oleg October 14th, 2010 @ 03:06 PM

    I'm not really familiar with other orms besides AR, but I see what you're saying. It works. Thanks for your help.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Referenced by

Pages