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 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 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 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 Apr 5, 2015
@curita
Copy link
Member

@curita 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 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

@pablohoffman pablohoffman commented Apr 7, 2015

needs rebase after merging #963

@nyov nyov force-pushed the remove-contrib branch from 32fe312 to 6d48c19 Apr 7, 2015
@nyov
Copy link
Contributor Author

@nyov nyov commented Apr 7, 2015

done

curita added a commit that referenced this issue Apr 10, 2015
[MRG+1] dissolve contrib_exp
@curita curita merged commit f8ca5d1 into scrapy:master Apr 10, 2015
1 check passed
@nyov nyov deleted the remove-contrib branch Apr 27, 2015
@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
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants