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

[MRG+1] FEED_EXPORT_FIELDS option #1159

Merged
merged 4 commits into from Apr 21, 2015
Merged

[MRG+1] FEED_EXPORT_FIELDS option #1159

merged 4 commits into from Apr 21, 2015

Conversation

@kmike
Copy link
Member

@kmike kmike commented Apr 14, 2015

As discussed in #1081, this option is nice to have if we're yielding items as dicts. This also fixes #1050.

There were no tests for FeedExporter class, so I've added some.

@kmike kmike added this to the Scrapy 1.0 milestone Apr 14, 2015
@@ -1,13 +1,15 @@
from __future__ import absolute_import

This comment has been minimized.

@pablohoffman

pablohoffman Apr 15, 2015
Member

just curious about why this is needed and whether changes to testproc.py are related to this pull request

This comment has been minimized.

@kmike

kmike Apr 15, 2015
Author Member

I went back and forth when writing FeedExporter tests; first tried to subclass tests from test_commands, then tried to use utilities from scrapy.utils.testproc, but in the end settled on another solution. That's why 973c31f happened.

from __future__ import absolute_import is always good to have, I'd add it to all Scrapy modules :) It makes imports compatible in Python 2.x and Python 3.x and prevents stupid errors when a file in the current folder shadows stdlib module or another module in sys.path.

This comment has been minimized.

@kmike

kmike Apr 15, 2015
Author Member

Should I remove the first commit?

This comment has been minimized.

@pablohoffman

pablohoffman Apr 15, 2015
Member

No, it's fine. I was just curious.

@pablohoffman
Copy link
Member

@pablohoffman pablohoffman commented Apr 15, 2015

looking good!

@pablohoffman pablohoffman changed the title FEED_EXPORT_FIELDS option [MRG+1] FEED_EXPORT_FIELDS option Apr 15, 2015
pablohoffman added a commit that referenced this pull request Apr 21, 2015
[MRG+1] FEED_EXPORT_FIELDS option
@pablohoffman pablohoffman merged commit 0a5bbba into master Apr 21, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@kmike kmike deleted the feed-export-fields branch Jul 29, 2015
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.

2 participants