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

Add 0.1 version of import_goodreads #105

Merged
merged 8 commits into from Sep 2, 2015
Merged

Add 0.1 version of import_goodreads #105

merged 8 commits into from Sep 2, 2015

Conversation

@jjconti
Copy link
Contributor

jjconti commented Sep 2, 2015

No description provided.

@jjconti
Copy link
Contributor Author

jjconti commented Sep 2, 2015

@ralsina

This comment has been minimized.

Copy link
Member

ralsina commented on dec2bab Sep 2, 2015

I have this feeling that if you required feed_parser and we found a way to inherit one plugin from the other, this plugin mostly goes away :-)

Juanjo Conti added 2 commits Sep 2, 2015
@jjconti
Copy link
Contributor Author

jjconti commented Sep 2, 2015

I thought of the idea (and I was going to open an issue) of having a generic import plugin from which this and other plugins can extend. It would not go away, just be refactored.

@ralsina
Copy link
Member

ralsina commented Sep 2, 2015

There is one, it's called BasicImport ImportMixin and you are already using it?

needs_config = False
doc_usage = "[options] rss_url"
doc_purpose = "import a Goodreads RSS"
cmd_options = [

This comment has been minimized.

Copy link
@ralsina

ralsina Sep 2, 2015

Member

ImportMixin already has cmd_options so you don't need to repeat them here.

This comment has been minimized.

Copy link
@jjconti

jjconti Sep 2, 2015

Author Contributor

I repeated it because I wanted to change the default value.

This comment has been minimized.

Copy link
@ralsina

ralsina Sep 2, 2015

Member

you probably don't want to default to "posts"... it will try to see if there's a "posts/conf.py" and do crazy stuff.


slug = utils.slugify(title)

# Needed becuase user_read_at can have a different locale

This comment has been minimized.

Copy link
@ralsina

ralsina Sep 2, 2015

Member

typo "becuase"

if item.get('user_review'):
content = item.get('user_review')

content += ("<br/><br/>" if content else "") + "Raiting: %s/5" % item.user_rating

This comment has been minimized.

Copy link
@ralsina

ralsina Sep 2, 2015

Member

Typo "Raiting"

Juanjo Conti
os.path.join(self.output_folder, slug + '.html'),
content)

@staticmethod

This comment has been minimized.

Copy link
@ralsina

ralsina Sep 2, 2015

Member

if you change post_date a few lines before, then you don't need this method at all

Juanjo Conti

It:

* users the date the user ends to read the book as post date

This comment has been minimized.

Copy link
@ralsina

ralsina Sep 2, 2015

Member

s/users/uses/

Juanjo Conti added 3 commits Sep 2, 2015
Juanjo Conti
Juanjo Conti
Juanjo Conti
jjconti added a commit that referenced this pull request Sep 2, 2015
Add 0.1 version of import_goodreads
@jjconti jjconti merged commit 37631cd into master Sep 2, 2015
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
for item in channel.entries:
self.process_item(item)

def process_item(self, item):

This comment has been minimized.

Copy link
@ralsina

ralsina Sep 2, 2015

Member

If you move the if into import_posts you don't need to reimplement process_item

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.