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

Use require_with_fallback in WordPress. #60

Closed
wants to merge 4 commits into from

Conversation

albertogg
Copy link
Member

My attempt to use #59 (a nicer LoadError messages) and apply them to both WordPress importers to solve #58.

@albertogg
Copy link
Member Author

I have to quote here, that both WordPress importers are working with jekyll import command. At least the did while testing it locally with bundler. If you guys can confirm this I'll do a pull request to the jekyll documentation.


module JekyllImport
# This importer takes a wordpress.xml file, which can be exported from your
# wordpress.com blog (/wp-admin/export.php).
module WordpressDotCom
def self.process(filename = {:source => "wordpress.xml"})
def self.process(options={})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change in this method seems unrelated. Could you provide a bit more info about why this is needed?

@albertogg
Copy link
Member Author

I changed that for consistency. The other wordpress.rb file is written like that, so I thought it was ok to keep consistency. Also, when you export a WordPress blog its never called wordpress.xml, so that default would not work any more, thats all.

If you prefer to keep it like that I'll revert that.

Revert self.process parameter to its original value.
wordpress.rb is the only importer that uses mysql2 gem, and the dependency is missing.
require File.join(File.dirname(__FILE__), "..", "..", "jekyll-import.rb")

required_gems = %w[rubygems sequel mysql2 fileutils safe_yaml]
JekyllImport.require_with_fallback(required_gems)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on

JekyllImport.require_with_fallback(%w[
  rubygems
  sequel
  mysql2
  fileutils
  safe_yaml
])

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this may be better, it looks more like a natural require!

@parkr
Copy link
Member

parkr commented Oct 27, 2013

This is a fantastic first pass!

@albertogg
Copy link
Member Author

Thanks!... I will correct it ASAP. In other words the jekyll import command for both WordPress importers (jekyll import wordpressdotcom --source wordpress.xml and jekyll import wordpress --dbname dbname --user root --pass root) are working as expected, so I think I can make a pull request in the jekyll documentation to make it "better"! have you try it yet?

Second pass fixes using parkr suggestions. this commit fixes jekyll#26 and fixes jekyll#58 issues.
@parkr parkr closed this in 208d39a Nov 8, 2013
@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
antonizoon pushed a commit to antonizoon/jekyll-import that referenced this pull request Jan 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants