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

Improve MovableType importer #13

Merged
merged 15 commits into from Mar 19, 2013

Conversation

Projects
None yet
3 participants
@ryangreenberg
Contributor

ryangreenberg commented Mar 3, 2013

I made a series of tweaks to the Jekyll importer for MovableType:

  • Use Sequel dataset instead of a hand-crafted SQL query. This makes it easier to access all the DB columns without specifying them directly.
  • Don't publish unpublished blog entries
  • Export file names with YYYY-MM-DD instead of non-padded dates.
  • Don't force hyphens in file names, which can break permalinks that used underscores.
  • Export excerpts for blog entries.

I'm happy to make any changes you'd like to see for this pull request. I have some tests for the importer stashed presently; I couldn't get rake test to run on master, so I thought I'd hold off for now.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 4, 2013

Member

Wow, these look great! I'd love to get those tests working – eventually we'd like all the migrators to be tested so we can release with peace-of-mind. What sort of problems were you having when running the tests?

Member

parkr commented Mar 4, 2013

Wow, these look great! I'd love to get those tests working – eventually we'd like all the migrators to be tested so we can release with peace-of-mind. What sort of problems were you having when running the tests?

@ryangreenberg

This comment has been minimized.

Show comment
Hide comment
@ryangreenberg

ryangreenberg Mar 4, 2013

Contributor

@parkr, thanks for the feedback. I'm more than happy to help add tests for the MT importer. I started looking at decomposing it into smaller pieces, which I think will make it easier to test and easier for people to override individual parts of the importer.

I was getting errors under 1.8.7 and 1.9.3 running rake test. Here the error under 1.8.7:

rake test --trace
rake aborted!
no such file to load -- rdoc/task
~/workspace/jekyll-import/Rakefile:64:in `require'
~/workspace/jekyll-import/Rakefile:64
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/rake_module.rb:25:in `load'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/rake_module.rb:25:in `load_rakefile'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:583:in `raw_load_rakefile'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:89:in `load_rakefile'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:160:in `standard_exception_handling'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:88:in `load_rakefile'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:72:in `run'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:160:in `standard_exception_handling'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:70:in `run'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/bin/rake:33
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/bin/rake:19:in `load'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/bin/rake:19
Contributor

ryangreenberg commented Mar 4, 2013

@parkr, thanks for the feedback. I'm more than happy to help add tests for the MT importer. I started looking at decomposing it into smaller pieces, which I think will make it easier to test and easier for people to override individual parts of the importer.

I was getting errors under 1.8.7 and 1.9.3 running rake test. Here the error under 1.8.7:

rake test --trace
rake aborted!
no such file to load -- rdoc/task
~/workspace/jekyll-import/Rakefile:64:in `require'
~/workspace/jekyll-import/Rakefile:64
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/rake_module.rb:25:in `load'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/rake_module.rb:25:in `load_rakefile'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:583:in `raw_load_rakefile'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:89:in `load_rakefile'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:160:in `standard_exception_handling'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:88:in `load_rakefile'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:72:in `run'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:160:in `standard_exception_handling'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:70:in `run'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/gems/rake-10.0.3/bin/rake:33
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/bin/rake:19:in `load'
~/.rbenv/versions/ree-1.8.7-2010.02/lib/ruby/gems/1.8/bin/rake:19
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 4, 2013

Member

Looks like we need to specify an rdoc dependency. If you add a development dependency in the gemspec for rdoc, does this resolve this error?

Member

parkr commented Mar 4, 2013

Looks like we need to specify an rdoc dependency. If you add a development dependency in the gemspec for rdoc, does this resolve this error?

@ryangreenberg

This comment has been minimized.

Show comment
Hide comment
@ryangreenberg

ryangreenberg Mar 9, 2013

Contributor

Once I specify the rdoc dependency I get the following error:

rake test --trace
** Invoke test (first_time)
** Execute test
~/.rbenv/versions/1.8.7-p358/bin/ruby -I"lib:test" -I"~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib" "~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/rake_test_loader.rb" "test/**/test_*.rb" 
rake aborted!
Command failed with status (1): [ruby -I"lib:test" -I"~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib" "~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/rake_test_loader.rb" "test/**/test_*.rb" ]
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/testtask.rb:104:in `define'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils.rb:45:in `call'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils.rb:45:in `sh'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:40:in `sh'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils.rb:82:in `ruby'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:40:in `ruby'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/testtask.rb:100:in `define'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:61:in `verbose'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/testtask.rb:98:in `define'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:228:in `call'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:228:in `execute'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:223:in `each'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:223:in `execute'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:166:in `invoke_with_call_chain'
~/.rbenv/versions/1.8.7-p358/lib/ruby/1.8/monitor.rb:242:in `synchronize'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:159:in `invoke_with_call_chain'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:152:in `invoke'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:143:in `invoke_task'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:101:in `top_level'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:101:in `each'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:101:in `top_level'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:110:in `run_with_threads'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:95:in `top_level'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:73:in `run'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:160:in `standard_exception_handling'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:70:in `run'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/bin/rake:33
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/bin/rake:19:in `load'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/bin/rake:19
Tasks: TOP => test

If you're able to run tests on master, I'll dig in deeper, otherwise we can fix whatever the issue is off master first.

Contributor

ryangreenberg commented Mar 9, 2013

Once I specify the rdoc dependency I get the following error:

rake test --trace
** Invoke test (first_time)
** Execute test
~/.rbenv/versions/1.8.7-p358/bin/ruby -I"lib:test" -I"~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib" "~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/rake_test_loader.rb" "test/**/test_*.rb" 
rake aborted!
Command failed with status (1): [ruby -I"lib:test" -I"~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib" "~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/rake_test_loader.rb" "test/**/test_*.rb" ]
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/testtask.rb:104:in `define'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils.rb:45:in `call'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils.rb:45:in `sh'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:40:in `sh'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils.rb:82:in `ruby'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:40:in `ruby'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/testtask.rb:100:in `define'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:61:in `verbose'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/testtask.rb:98:in `define'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:228:in `call'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:228:in `execute'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:223:in `each'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:223:in `execute'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:166:in `invoke_with_call_chain'
~/.rbenv/versions/1.8.7-p358/lib/ruby/1.8/monitor.rb:242:in `synchronize'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:159:in `invoke_with_call_chain'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/task.rb:152:in `invoke'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:143:in `invoke_task'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:101:in `top_level'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:101:in `each'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:101:in `top_level'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:110:in `run_with_threads'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:95:in `top_level'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:73:in `run'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:160:in `standard_exception_handling'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/lib/rake/application.rb:70:in `run'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/gems/rake-10.0.3/bin/rake:33
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/bin/rake:19:in `load'
~/.rbenv/versions/1.8.7-p358/lib/ruby/gems/1.8/bin/rake:19
Tasks: TOP => test

If you're able to run tests on master, I'll dig in deeper, otherwise we can fix whatever the issue is off master first.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 10, 2013

Member

Getting the same on master at the moment, hmm. I'll dig into it.

Member

parkr commented Mar 10, 2013

Getting the same on master at the moment, hmm. I'll dig into it.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 10, 2013

Member

Ok, tests are running for me again on master. @mojombo made a bunch of changes and must not have had the time to fix up the tests before he pushed them up. Please rebase on master and git push origin improve_mt_migrator --force once you have your tests written. Thank you!

Member

parkr commented Mar 10, 2013

Ok, tests are running for me again on master. @mojombo made a bunch of changes and must not have had the time to fix up the tests before he pushed them up. Please rebase on master and git push origin improve_mt_migrator --force once you have your tests written. Thank you!

@ryangreenberg

This comment has been minimized.

Show comment
Hide comment
@ryangreenberg

ryangreenberg Mar 10, 2013

Contributor

@parkr, thanks for your help. Tests worked fine once I pulled in @mojombo's latest master. I added tests that cover a lot of the functionality in the MovableType importer in ec2f780 and 0f709ed. Let me know if you'd like any other changes.

Contributor

ryangreenberg commented Mar 10, 2013

@parkr, thanks for your help. Tests worked fine once I pulled in @mojombo's latest master. I added tests that cover a lot of the functionality in the MovableType importer in ec2f780 and 0f709ed. Let me know if you'd like any other changes.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 14, 2013

Member

Awesome! Thanks, Ryan. I'll take a look at these later today :-)

Member

parkr commented Mar 14, 2013

Awesome! Thanks, Ryan. I'll take a look at these later today :-)

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 19, 2013

Member

This rocks.

Member

parkr commented Mar 19, 2013

This rocks.

parkr added a commit that referenced this pull request Mar 19, 2013

@parkr parkr merged commit f15262e into jekyll:master Mar 19, 2013

parkr added a commit that referenced this pull request Mar 19, 2013

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

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