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

Update migration.rb to no more use `alias_method_chain` which is deprecated with Rails 5. #44

Merged
merged 1 commit into from Sep 11, 2017

Conversation

Projects
None yet
7 participants
@guille-moe
Collaborator

guille-moe commented May 19, 2017

Simply replace alias_method_chain with alias_method.

floehopper added a commit to alphagov/manuals-publisher that referenced this pull request May 25, 2017

Upgrade mongoid_rails_migrations gem to v1.1.0+
This upgrade from v1.0.0 includes a few changes to improve compatibility
with more recent versions of Mongoid and Rails. However, I haven't been
able to use the v1.1.0 release version of the gem for three reasons:

* Since we're now on Mongoid v6, we need the fix [1] in the latest
commit on master in the canonical repo in order for
`Mongoid::Migration.connection` method to work. I've incorporated this
commit in the new branch in the alphagov fork.

* Since we're now on Rails v5, we need a fix [2] for the absence of
`alias_method_chain`. I've incorporated the commit from this PR [3] into
the new branch in the alphagov fork.

* The problem described in #816 still exists. The maintainer of the gem
still hasn't fixed the problem. This earlier suggested fix [4] has been
closed without explanation. A new fix [5] has been proposed and I've
added a comment to explain why I think it should be merged. Therefore
I've incorporated my original fix as the last commit in the new branch
in the alphagov fork.

I've checked that I can generate and run the following migration:

    class Foo < Mongoid::Migration
      def self.up
        p SectionEdition.count
        p SectionEdition.collection
        p connection.database.collection_names
      end

      def self.down
      end
    end

I've also updated the existing migration so that I think they would
work. However, I haven't tested them.

[1]: adacosta/mongoid_rails_migrations@0074dbf
[2]: stormz/mongoid_rails_migrations@b9158cd
[3]: adacosta/mongoid_rails_migrations#44
[4]: adacosta/mongoid_rails_migrations#3
[5]: adacosta/mongoid_rails_migrations#40
@skalee

This comment has been minimized.

skalee commented Jun 12, 2017

This is the simplest thing to fix Rails 5 compatibility. It's a true equivalent of inlining the alias_method_chain.

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Aug 17, 2017

Any chance to get this merged? Rails 5.1 is already out for a while and totally removed alias_method_chain so it doesn't work any more.

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Sep 10, 2017

Please don't let this project die 😞

@adacosta

This comment has been minimized.

Owner

adacosta commented Sep 11, 2017

@jarthod let me know if all is well in master. If so, I'll tag and push an updated version.

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Sep 11, 2017

@adacosta confirmed! all is well in master 👍 Thanks!

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Dec 9, 2017

@adacosta could you please push a new version to rubygems? thanks !

@Aethelflaed

This comment has been minimized.

Aethelflaed commented Jan 10, 2018

Could you push a new version to rubygems? Thanks!

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jan 28, 2018

Still no update 😢

@ajsharp

This comment has been minimized.

ajsharp commented May 31, 2018

@adacosta Any update on this? I'm sure someone here would be willing to help maintain this if you need some help.

@CyberMew

This comment has been minimized.

CyberMew commented Jul 25, 2018

Is there an alternative? It's been more than 3 years since last release, and I'm not sure if this is still working with the latest mongoid release?

@adacosta

This comment has been minimized.

Owner

adacosta commented Jul 25, 2018

I think there are better maintained forks as an alternative. If someone would like to take over maintenance, ping me.

@guille-moe

This comment has been minimized.

Collaborator

guille-moe commented Jul 25, 2018

@adacosta I think, starting by releasing a 1.1.1 of the gem would be a good start to support latest Rails versions, because of no release, we currently use our own fork based on this PR branch (already merged as you see) with Rails 5.2 and Mongoid 6.3.
I don't know why no release as been done for years, is master broken ? Well, if I can do anything to help you to progress on the release tell me.
Feel free to tag me on issues too.

@adacosta

This comment has been minimized.

Owner

adacosta commented Jul 25, 2018

@guille-moe I added you as a collaborator. Releases haven't been done for years because I haven't needed to use this project for years. Feel free to do whatever is needed.

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jul 25, 2018

@adacosta I can help if needed too, but in order @guille-moe or me to do a release, you'll need to give one of us ownership on rubygems (https://rubygems.org/gems/mongoid_rails_migrations), using (for me):

gem owner --add me@adrienjarthon.com mongoid_rails_migrations

Thanks.

@adacosta

This comment has been minimized.

Owner

adacosta commented Jul 25, 2018

done.

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jul 25, 2018

Thanks, I'll try to ship this release this week.
@guille-moe I can add you to rubygems too if you want to do it.
@adacosta would you like to keep an eye on merge requests and changes to this repo or you don't care any more?

@adacosta

This comment has been minimized.

Owner

adacosta commented Jul 25, 2018

@jarthod feel free to maintain at a level you see fit.

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jul 28, 2018

Ok guys I've juste released 1.1.1 with current master (sorry the date is wrong on rubygems, due to hardcoding it in gemspec probably), I'm gonna work on adding some multi-version testing with Travis and cleanup the old code a bit for 1.2.0. If you have fork with interesting changes, please open a pull-request.

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jul 29, 2018

@adacosta could you please give me admin permission on the repo so I can setup Travis CI ?

@adacosta

This comment has been minimized.

Owner

adacosta commented Jul 29, 2018

@jarthod Does that require this repo to be under an organization? If there isn't a quick/easy way, you can PM me the travis user name and token you would like enabled. I see the config section.

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jul 29, 2018

@adacosta I don't think it does, Travis just says:

To start using Travis CI, make sure you have all of the following:
GitHub login
Admin permissions for a project hosted on GitHub
Working code in your project
Working build or test script

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jul 29, 2018

I see you've enabled Travis 4 years ago from your account: https://travis-ci.org/adacosta/mongoid_rails_migrations/builds but it's probably disabled now as it doesn't build any more. You can probably just enable it again I guess.

@adacosta

This comment has been minimized.

Owner

adacosta commented Jul 29, 2018

The github config page raises an error, "There was an error updating your hook: These events are not allowed for this hook: membership" if I try to update the travis setting but maybe it should be a webhook anyway ? https://developer.github.com/changes/2018-04-25-github-services-deprecation/ . In regard to giving you admin, I can't find a place in the UI to delegate and the googles only mentions of this feature in organizations except for old search results.

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jul 29, 2018

Mm ok I wasn't aware of this change, It seems they are migrating to Github Apps on travis-ci.com only and are merging open source projects here: https://docs.travis-ci.com/user/open-source-on-travis-ci-com/ (they will migrate projects gradually from dot org to dot com)

Could you please try signing in with https://travis-ci.com to enable the project here? (no paying subscription required of course)

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jul 29, 2018

@adacosta Ok this is more complicated, if the project has even been active on dot org you can't activate it on dot com yet, you need to wait for the official migration or contact them:

Repositories may also be migrated without their build history or build settings (including environment variables) immediately. Please contact support support@travis-ci.com and include “Repository Migration” somewhere in the subject line, and the name of the repository in your email.

So unless you're feeling like contacting them don't bother for now, I'll configure travis with a fork on my account and we'll see later for the migration (should have started Q2 2018 they said)

@jarthod

This comment has been minimized.

Collaborator

jarthod commented Jul 29, 2018

Ok it's working fine and travis-ci.org actually started building when I pushed on master so it wasn't disabled after all. We now have a decent test matrix on the repo and the badge in the readme: https://travis-ci.org/adacosta/mongoid_rails_migrations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment