Skip to content

Conversation

@thii
Copy link
Contributor

@thii thii commented Aug 17, 2016

Instead of putting an empty string, leaving it out and iOS will automaticallly
choose the closest available localizable string.

Instead of putting an empty string, leaving it out and iOS will automaticallly
choose the closest available localizable string.
if row_value == ""
entry = ""
else
entry = comment.to_s.empty? ? "" : "\n/* #{comment} */\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Trailing whitespace detected.

@netbe
Copy link
Owner

netbe commented Aug 22, 2016

@thii thanks for the contribution, could you explain more about your PR, I am not sure to understand the gain here?

And then to be accepted:

  1. Write a test case
  2. Make sure the all the tests pass: right now this feature breaks following test:
Failure: test_converting_csv_to_dotstrings_with_no_default_lang_is_empty(TestCSV2Strings)
/Users/netbe/Code/Babelish/test/babelish/test_csv2strings.rb:53:in `test_converting_csv_to_dotstrings_with_no_default_lang_is_empty'
     50:     converter.convert
     51:     assert File.exist?(spanish_file), "the ouptut file does not exist"
     52:     result = File.read(spanish_file)
  => 53:     assert_equal expected_output, result
     54:     system("rm -rf *.lproj")
     55:   end
     56:
<"\"GREETINGS\"      = \"Buenos dias\";\n\"ANOTHER_STRING\" = \"\";\n"> expected but was
<"\"GREETINGS\"      = \"Buenos dias\";\n">

diff:
  "GREETINGS"      = "Buenos dias";
- "ANOTHER_STRING" = "";
  1. Could you fix the hound violations ?

Thank you

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 47d451a on thii:empty-string into * on netbe:master*.

@netbe netbe added the iOS label Aug 22, 2016
@netbe
Copy link
Owner

netbe commented Aug 22, 2016

please rebase with master so tests can run on travis

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.

4 participants