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

Refactor (remove duplicate code) #311

merged 3 commits into from Jan 8, 2016


Copy link

@akx akx commented Dec 30, 2015

This PR is directly inspired by #62.

A little bit of intricate, fine magic is used to create an Optparse parser out of the Distutils commands, a little bit of fine-tuning here and there and then everything . . . just works, with a third less code.

The ,fuzzy changes in the tests are actually an oversight that this PR fixes as a side effect: The commit e85e71d had added catalog.fuzzy = False to the distutils command code, but the same change had never made it into the CLI code.

This should do wonders for test coverage statistics, which also means the frontend code will be more easily maintained.

@akx akx added the enhancement label Dec 30, 2015
@gitmate-bot gitmate-bot added the size/XL label Dec 30, 2015
@akx akx force-pushed the akx:refactor-frontend branch 2 times, most recently from 6fc7d0c to 0229d4c Dec 30, 2015
Copy link

@codecov-io codecov-io commented Dec 30, 2015

Current coverage is 88.28%

Merging #311 into master will increase coverage by +4.02% as of 86c1158

@@            master    #311   diff @@
  Files           22      22       
  Stmts         3776    3595   -181
  Branches         0       0       
  Methods          0       0       
- Hit           3182    3174     -8
  Partial          0       0       
+ Missed         594     421   -173

Review entire Coverage Diff as of 86c1158

Powered by Codecov. Updated on successful CI builds.

@akx akx added this to the Babel 2.3/3.0 milestone Jan 3, 2016
@akx akx force-pushed the akx:refactor-frontend branch from f63687e to fd22a8e Jan 4, 2016
Copy link
Member Author

@akx akx commented Jan 8, 2016

@sils1297 Feel free to merge this..?

Copy link

@sils sils commented Jan 8, 2016

@akx I don't have time to do a full review :/ it seems well tested and a good improvement, if you're confident, feel free to merge

Copy link
Member Author

@akx akx commented Jan 8, 2016

I'm pretty confident. Let's see how it goes :)

akx added a commit that referenced this pull request Jan 8, 2016
Refactor (remove duplicate code)
@akx akx merged commit 1b4cd01 into python-babel:master Jan 8, 2016
5 of 6 checks passed
5 of 6 checks passed
review/gitmate/manual This commit needs review.
Scrutinizer 9 updated code elements
codecov/project 88.28% (+4.02%) compared to 3aa3f29 at 84.26%
continuous-integration/appveyor/pr AppVeyor build succeeded
continuous-integration/travis-ci/pr The Travis CI build passed
review/gitmate/commit All is well! :)
@sils sils removed the (3) pending review label Jan 8, 2016
@akx akx deleted the akx:refactor-frontend branch Jan 8, 2016
akx added a commit to akx/babel that referenced this pull request Apr 22, 2016
This is a combination of the test suite improvement fbc1648 and the frontend changes in 414aec5..ee8abd6.

* Harmonize extraction keyword parsing between distutils and standalone CLI (python-babel#388, python-babel#384, python-babel#311)
* Don't use unicode-variant %r for logging
* extract: don't die badly when no input paths are specified in optparse mode
* Remind the optparse CLI about `extract -s` (a shorthand for `--strip-comments`) (python-babel#390)
* Teach the optparse CLI about the parameter aliases it had forgotten in python-babel#311 (python-babel#390)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants