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

joomla: use & require mysql2 instead of mysql #255

Merged
merged 1 commit into from Jul 14, 2016

Conversation

Projects
None yet
4 participants
@alejandroerickson
Contributor

alejandroerickson commented May 25, 2016

joomla.rb was using Sequel.mysql, which appears to depend on the gem mysql. This is depracated and replacing mysql with mysql2 works fine; it is already in use for wordpress.rb. I also added the requirement for the mysql2 gem, as this doesn't appear to work without that.

Details of my error message when running the vanilla joomla importer without mysql:

/System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require': LoadError: cannot load such file -- mysql (Sequel::AdapterNotFound)
        from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/adapters/mysql.rb:6:in `rescue in <top (required)>'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/adapters/mysql.rb:3:in `<top (required)>'
        from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/database/connecting.rb:98:in `load_adapter'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/database/connecting.rb:28:in `adapter_class'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/database/connecting.rb:56:in `connect'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/core.rb:108:in `connect'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/core.rb:377:in `adapter_method'
        from /Library/Ruby/Gems/2.0.0/gems/sequel-4.34.0/lib/sequel/core.rb:387:in `mysql'
        from /Library/Ruby/Gems/2.0.0/gems/jekyll-import-0.10.0/lib/jekyll-import/importers/joomla.rb:38:in `process'
        from /Library/Ruby/Gems/2.0.0/gems/jekyll-import-0.10.0/lib/jekyll-import/importer.rb:23:in `run'
        from -e:2:in `<main>

`

I suggest, furthermore, that we say MySQL (not the gem) needs to be installed, which is not obvious to people who rent their webserver and database from a third party (i.e., me! heh). I'm not sure how to edit the documentation for joomla.rb, but it definitely needs to go onto individual importer pages, since a user of X CMS will not be looking over the documentation for Y CMS necessarily.

This is my first ever pull request so any comments on how I've done it are welcome.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 14, 2016

Member

LGTM!

Member

parkr commented Jul 14, 2016

LGTM!

@parkr parkr changed the title from use mysql2 and require it in joomla to joomla: use & require mysql2 instead of mysql Jul 14, 2016

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 14, 2016

Member

@jekyllbot: merge

Member

parkr commented Jul 14, 2016

@jekyllbot: merge

@jekyllbot jekyllbot merged commit 8409bdb into jekyll:master Jul 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

jekyllbot added a commit that referenced this pull request Jul 14, 2016

@ienev

This comment has been minimized.

Show comment
Hide comment
@ienev

ienev May 23, 2017

Contributor

Looks like the same needs to be done for the Joomla3 converter. I had to recently do some imports and had to manually apply the changes. Is there a specific reason for this not being updated or should I proceed with a PR?

Also I think it's worth adding a port option as there are cases (mine included) where mysql listens on a non-standard port.

Contributor

ienev commented May 23, 2017

Looks like the same needs to be done for the Joomla3 converter. I had to recently do some imports and had to manually apply the changes. Is there a specific reason for this not being updated or should I proceed with a PR?

Also I think it's worth adding a port option as there are cases (mine included) where mysql listens on a non-standard port.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr May 23, 2017

Member

@ienev I'd be happy to take a look at a PR if you're willing to put one together! Unfortunately I don't have much time to put one together myself.

Member

parkr commented May 23, 2017

@ienev I'd be happy to take a look at a PR if you're willing to put one together! Unfortunately I don't have much time to put one together myself.

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