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] [image & file pipeline] loading setting for user classes #2243

Merged
merged 1 commit into from Sep 19, 2016

Conversation

@pawelmhm
Copy link
Contributor

@pawelmhm pawelmhm commented Sep 15, 2016

if user has some custom subclass of Image pipeline and no setting for
this pipeline, he should get default settings defined for Image Pipeline.

Fixes #2198

if user has some custom subclass of Image pipeline and no setting for
this pipeline, he should get default settings defined for Image Pipeline.

Fixes #2198
@pawelmhm pawelmhm force-pushed the pawelmhm:image-pipeline-2198 branch from 049a081 to 7d88209 Sep 15, 2016
@codecov-io
Copy link

@codecov-io codecov-io commented Sep 15, 2016

Current coverage is 83.36% (diff: 100%)

Merging #2243 into master will increase coverage by <.01%

Powered by Codecov. Last update 129421c...7d88209

@redapple
Copy link
Contributor

@redapple redapple commented Sep 15, 2016

@briehanlombaard , would you be able to test the fix on your end too?

@briehanlombaard
Copy link

@briehanlombaard briehanlombaard commented Sep 15, 2016

@redapple That fixes the problem. Thanks!

@redapple redapple changed the title [image & file pipeline] loading setting for user classes [MRG+1] [image & file pipeline] loading setting for user classes Sep 15, 2016
@redapple redapple added this to the v1.1.3 milestone Sep 15, 2016
@redapple
Copy link
Contributor

@redapple redapple commented Sep 15, 2016

Thank you for checking @briehanlombaard (and reporting before that!)

Copy link
Contributor

@redapple redapple left a comment

Thanks @pawelmhm !

@redapple
Copy link
Contributor

@redapple redapple commented Sep 19, 2016

@kmike @eliasdorneles , do you agree with the change?

@eliasdorneles
Copy link
Member

@eliasdorneles eliasdorneles commented Sep 19, 2016

Yup, looks good to me.
It's a bit tricky to follow the code (as many things related to the media pipeline), but the new behavior is better.

@redapple redapple merged commit 41cd9f4 into scrapy:master Sep 19, 2016
2 checks passed
2 checks passed
codecov/patch 100% of diff hit (target 100%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
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

5 participants