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] CSS support in link extractors #983

Merged
merged 4 commits into from Mar 17, 2015
Merged

[MRG+1] CSS support in link extractors #983

merged 4 commits into from Mar 17, 2015

Conversation

@ArturGaspar
Copy link
Contributor

@ArturGaspar ArturGaspar commented Dec 12, 2014

This adds support for using CSS selectors in link extractors.

@kmike
Copy link
Member

@kmike kmike commented Dec 13, 2014

I like this feature.

My only concern is backwards compatibility: a new option is inserted in the middle, so user code that uses positional arguments will be broken.

Using positional arguments is a bad style when list of arguments is long, but we're using it in Scrapy code ourselves (see super() calls).

+1 to merge this PR after converting link extractor super() calls to use keyword arguments.

… for backwards compatibility, use keyword arguments in link extractor super().__init__() calls
@ArturGaspar
Copy link
Contributor Author

@ArturGaspar ArturGaspar commented Dec 15, 2014

I have moved the new argument to the end, and changed the link extractor classes to use keyword arguments in super().__init__().

@ArturGaspar
Copy link
Contributor Author

@ArturGaspar ArturGaspar commented Dec 22, 2014

It seems the Travis CI build has failed because of a timeout somewhere not related to my changes.
In my computer, the tests passed.

@kmike kmike changed the title CSS support in link extractors [MRG+1] CSS support in link extractors Mar 14, 2015
@kmike
Copy link
Member

@kmike kmike commented Mar 14, 2015

Looks good to me, +1 to merge.

pablohoffman added a commit that referenced this pull request Mar 17, 2015
[MRG+1] CSS support in link extractors
@pablohoffman pablohoffman merged commit f924567 into scrapy:master Mar 17, 2015
1 check passed
1 check passed
continuous-integration/travis-ci 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

3 participants