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

Fix drupal6-to-jekyll migrations #42

Merged
merged 4 commits into from Jul 16, 2013

Conversation

Projects
None yet
4 participants
@jinghao
Contributor

jinghao commented Jul 15, 2013

This addresses issues 2 and 3 as identified in 0b33a12#commitcomment-3631300

Instead of manually serializing the tags/categories list as a string ourselves, we leave it as an array and let to_yaml handle the serialization.

I tested this on my drupal installation with posts that contain tags that contain and don't contain spaces, and with posts that contain and don't contain tags.

@@ -59,7 +59,8 @@ def self.process(dbname, user, pass, host = 'localhost', prefix = '')
node_id = post[:nid]
title = post[:title]
content = post[:body]
tags = post[:tags].downcase.strip

This comment has been minimized.

@jinghao

jinghao Jul 15, 2013

Contributor

This is because the LEFT OUTER JOIN leaves some posts without tags. It gets turned into nil instead of the expected empty string.

@jinghao

jinghao Jul 15, 2013

Contributor

This is because the LEFT OUTER JOIN leaves some posts without tags. It gets turned into nil instead of the expected empty string.

@jinghao

This comment has been minimized.

Show comment
Hide comment
@jinghao

jinghao Jul 15, 2013

Contributor

Cool, updated to reflect the style request:

tags = post[:tags].downcase.strip unless post[:tags].nil?
Contributor

jinghao commented Jul 15, 2013

Cool, updated to reflect the style request:

tags = post[:tags].downcase.strip unless post[:tags].nil?
@jinghao

This comment has been minimized.

Show comment
Hide comment
@jinghao

jinghao Jul 15, 2013

Contributor

I like post.fetch(:tags, '') much better

Contributor

jinghao commented Jul 15, 2013

I like post.fetch(:tags, '') much better

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 15, 2013

Member

LGTM! @mattr-?

Member

parkr commented Jul 15, 2013

LGTM! @mattr-?

@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Jul 15, 2013

Member

:+:1

Member

mattr- commented Jul 15, 2013

:+:1

@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Jul 15, 2013

Member

Well, that was supposed to be 👍 bit you get the idea

Member

mattr- commented Jul 15, 2013

Well, that was supposed to be 👍 bit you get the idea

@jinghao

This comment has been minimized.

Show comment
Hide comment
@jinghao

jinghao Jul 16, 2013

Contributor

👍 (this is new)

Anyone want to approve? :)

Contributor

jinghao commented Jul 16, 2013

👍 (this is new)

Anyone want to approve? :)

mattr- added a commit that referenced this pull request Jul 16, 2013

Merge pull request #42 from jinghao/master
Fix drupal6-to-jekyll migrations

@mattr- mattr- merged commit deca4cf into jekyll:master Jul 16, 2013

1 check passed

default The Travis CI build passed
Details
@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Jul 16, 2013

Member

Thanks for improving our Drupal 6 importer! ❤️

Member

mattr- commented Jul 16, 2013

Thanks for improving our Drupal 6 importer! ❤️

@jinghao

This comment has been minimized.

Show comment
Hide comment
@jinghao

jinghao Jul 16, 2013

Contributor

No problem! I hope it'll save other people time.

Contributor

jinghao commented Jul 16, 2013

No problem! I hope it'll save other people time.

mattr- added a commit that referenced this pull request Jul 16, 2013

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 16, 2013

Member

👍 🎉 🎆

Member

parkr commented Jul 16, 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.