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

Revert "import_cldr: if lxml is installed, use it" #393

Merged
merged 1 commit into from
Apr 19, 2016

Conversation

jtwang
Copy link
Contributor

@jtwang jtwang commented Apr 19, 2016

This reverts commit 9b5c7f3.

Addresses #391, not much else to say. :) FYI @aronbierbaum

@akx
Copy link
Member

akx commented Apr 19, 2016

Agh, @jtwang, can you do git commit --amend --no-edit and re-force-push to restart builds?

AppVeyor's panties got in a random network bunch and I can't merge this without CI passing :D

@aronbierbaum
Copy link

aronbierbaum commented Apr 19, 2016

Thank you for addressing this so quickly.

If you wanted to continue using lxml you might be able to do something like:

parse = lxml.etree.XMLParser(remove_comments = True).parse

@jtwang jtwang force-pushed the 391_jtwang_revert_lxml_optimization branch from ebc05ac to 670a84e Compare April 19, 2016 14:41
@jtwang
Copy link
Contributor Author

jtwang commented Apr 19, 2016

@akx force pushed

yoda force push

@aronbierbaum - Thanks for poking at the problem! Since Aarni is OK with nuking his addition, I'm going to stick with removing lxml entirely in the interests for KISS. :)

@codecov-io
Copy link

Current coverage is 90.08%

Merging #393 into master will not affect coverage as of 4845ef1

@@            master    #393   diff @@
======================================
  Files           24      24       
  Stmts         3935    3935       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit           3545    3545       
  Partial          0       0       
  Missed         390     390       

Review entire Coverage Diff as of 4845ef1

Powered by Codecov. Updated on successful CI builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants