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] dissolve contrib_exp #1134

Merged
merged 1 commit into from
Apr 10, 2015
Merged

[MRG+1] dissolve contrib_exp #1134

merged 1 commit into from
Apr 10, 2015

Conversation

nyov
Copy link
Contributor

@nyov nyov commented Apr 3, 2015

Here's a start for #1063, getting rid of contrib_exp.
I haven't deleted the old xml_iter yet, I think it should be done in the next commit so we have at least a point in history where bot are available.
The decompression middleware I moved to contrib, django item was only a deprecation warning anyway.

edit: I don't think we need backwards-compat. for this one, it said "experimental" for a reason.

@kmike
Copy link
Member

kmike commented Apr 3, 2015

  1. What about keeping both xmliter_lxml and xmliter for now? Before removing xmliter we should check that xmliter_lxml works better.
  2. Even though contrib_exp was experimental, people used it, so +1 from me to add a backwards compatibility shim.

@nyov
Copy link
Contributor Author

nyov commented Apr 3, 2015

Not my decision to remove xmliter, I wouldn't mind to keep both, @curita mentioned that in #1063 (comment).

On the compat shims, would you agree that we don't need one for djangoitem? It was already a shim (since 70f8e51).

@kmike
Copy link
Member

kmike commented Apr 3, 2015

On the compat shims, would you agree that we don't need one for djangoitem? It was already a shim (since 70f8e51).

Yes, +1 to dropping the existing djangoitem shim.

@nyov nyov force-pushed the remove-contrib branch 2 times, most recently from 08bd13c to 32fe312 Compare April 5, 2015 18:43
@curita
Copy link
Member

curita commented Apr 7, 2015

I don't recall why we decided on removing xmliter really (/cc @nramirezuy, @pablohoffman). I'm OK with keeping it too.

@kmike
Copy link
Member

kmike commented Apr 7, 2015

Thanks @nyov, the PR looks good to me.
+1 to merge it after #963.

@kmike kmike changed the title dissolve contrib_exp [MRG+1] dissolve contrib_exp Apr 7, 2015
@pablohoffman
Copy link
Member

needs rebase after merging #963

@nyov
Copy link
Contributor Author

nyov commented Apr 7, 2015

done

curita added a commit that referenced this pull request Apr 10, 2015
[MRG+1] dissolve contrib_exp
@curita curita merged commit f8ca5d1 into scrapy:master Apr 10, 2015
@nyov nyov deleted the remove-contrib branch April 27, 2015 19:06
@curita curita added this to the Scrapy 1.0 milestone May 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants