Czech language support fixed and improved#148
Conversation
foton
changed the title
1 similar comment
jlduran
left a comment
jlduran
left a comment
There was a problem hiding this comment.
Thank you for taking the time to revisit this one.
I have made a few suggestions, once these are addressed, I will squash the commits and merge!
| end | ||
| end | ||
|
|
||
| %i[megs thousands].each do |method_name| |
There was a problem hiding this comment.
megs is also defined below.
In order to achieve 100% coverage, the thousands method should be removed/tested.
| end | ||
|
|
||
| it 'should pluralize correctly' do | ||
| @backend.send(:pluralize, :cs, @hash, 1).should eq('one') |
There was a problem hiding this comment.
Please use the new:expect syntax:
expect(@backend.send(:pluralize, :cs, @hash, 1)).to eq('one')https://travis-ci.org/kslazarev/numbers_and_words/jobs/262259408#L197
| @@ -0,0 +1,32 @@ | |||
| to_words: | |||
| # simple_example: | |||
There was a problem hiding this comment.
Please uncomment/remove these specs.
| - jedno sto jedna | ||
| - dvacet jedna | ||
| - třináct | ||
| # complex_example_with_options: |
There was a problem hiding this comment.
Please uncomment/remove these specs.
| # - 0.31 | ||
| # - 0.12 | ||
| # : | ||
| # - nula celá jedna desetina |
There was a problem hiding this comment.
I guess, with options: :remove_zero, this should be just jedna desetina.
| @current_capacity ||= 0 | ||
| # @current_capacity = order of block of 3 digits, backwards | ||
|
|
||
| case @current_capacity |
There was a problem hiding this comment.
I guess this case could be replaced with a multiple if statement, for instance:
if @current_capacity.zero?
options.gender.result
elsif @current_capacity > 2 && @current_capacity.odd?
:female
else
:male
end| 0.77: nula celých sedmdesát sedm setin | ||
| 0.777: nula celých sedm set sedmdesát sedm tisícin | ||
| 0.7777: nula celých sedm tisíc sedm set sedmdesát sedm desetitisícin | ||
| 0.293517431: nula celých dvě stě devadesát tři milióny pět set sedmnáct tisíc čtyři sta třicet jedna miliardtina |
There was a problem hiding this comment.
In order to get 100% coverage, we need a test like this, for example:
0.21021: nula celých dvacet jeden tisíc dvacet jedna stotisícinahttps://travis-ci.org/kslazarev/numbers_and_words/jobs/262259408#L214
| 36: třicátá šestá | ||
| 100: stá | ||
| 999: devítistá devadesátá devátá | ||
|
|
There was a problem hiding this comment.
Please remove this extra line.
| t([options[:prefix], :hundreds, gender_to_use(options)].join('.'))[number] | ||
| end | ||
|
|
||
| def thousands(number, options = {}) |
There was a problem hiding this comment.
In order to achieve 100% coverage, this method should be removed/tested.
|
@foton want to finish this? |
|
@dblock it's OK to merge as is. I can address the suggestions on the weekend. |
|
I see now, ignore me. I think the change requests should be addressed before merging. |
|
I will fix it today |
1 similar comment
|
Fixed and coverage is back to 100%. |
|
Thank you @foton! |
|
Closes #123 |
Release 0.11.2 (August 14, 2017) - Add full support for Czech language. [#148]
5 participants
Continued on work from #123.