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

Fix setup.py option 'add_location' #459

Merged
merged 1 commit into from
Jan 25, 2017

Conversation

oleksandr-kuzmenko
Copy link
Contributor

Fix bug in setup.py options.

@codecov-io
Copy link

codecov-io commented Dec 15, 2016

Current coverage is 90.14% (diff: 100%)

Merging #459 into master will not change coverage

@@             master       #459   diff @@
==========================================
  Files            24         24          
  Lines          3979       3979          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           3587       3587          
  Misses          392        392          
  Partials          0          0          

Powered by Codecov. Last update ab56a43...9be702b

@@ -277,7 +277,7 @@ class extract_messages(Command):
'path to the mapping configuration file'),
('no-location', None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not add it for this and the others?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because there are different types of options: no-location is boolean option, add-location is multiple value option

example which does't work without this fix:

python setup.py extract_messages \
--mapping-file international/babel_mapping.ini \
--output-file proj/i18n/file.pot \
--charset utf-8 --add-location file \
--omit-header --sort-output

omit-header and sort-output -- boolean options, output-file and add-location -- multiple value options

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sils so is it OK for you?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, yes. Too much going on :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sils can you accept PR?

@sils
Copy link
Member

sils commented Jan 25, 2017

ack 9be702b

@sils sils merged commit 16b3a32 into python-babel:master Jan 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants