-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Random words to add should be 0 #719
Random words to add should be 0 #719
Conversation
Gives very unexpected results, especially weird when validating max char length of a field
a359def
to
a5d7731
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Just left a few comments. Take a look and let me know your thoughts.
ps: I already updated your branch with master.
lib/faker/lorem.rb
Outdated
def sentence(word_count = 4, supplemental = false, random_words_to_add = 6) | ||
words(word_count + rand(random_words_to_add.to_i), supplemental).join(' ').capitalize + locale_period | ||
def sentence(word_count = 4, supplemental = false, random_words_to_add = 0) | ||
words(word_count + rand(random_words_to_add.to_i), supplemental).join(' ').capitalize + '.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't do this change, because the period punctuation is different in a few languages. That's why we have a locale_period
private method. I'd suggest undoing this change and we can merge the rest @swapsCAPS
lib/faker/lorem.rb
Outdated
def question(word_count = 4, supplemental = false, random_words_to_add = 6) | ||
words(word_count + rand(random_words_to_add.to_i), supplemental).join(locale_space).capitalize + locale_question_mark | ||
def question(word_count = 4, supplemental = false, random_words_to_add = 0) | ||
words(word_count + rand(random_words_to_add.to_i).to_i, supplemental).join(' ').capitalize + '?' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't do this change as well.
"In Greek, which uses the Greek alphabet, the question mark ";"is identical to the Latin semicolon: Πώς σας λένε;" credits to Quora
lib/faker/lorem.rb
Outdated
def paragraph(sentence_count = 3, supplemental = false, random_sentences_to_add = 3) | ||
sentences(resolve(sentence_count) + rand(random_sentences_to_add.to_i), supplemental).join(locale_space) | ||
def paragraph(sentence_count = 3, supplemental = false, random_sentences_to_add = 0) | ||
sentences(resolve(sentence_count) + rand(random_sentences_to_add.to_i), supplemental).join(' ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Space is space everywhere. I agree 👍
ad7704c
to
dc4d3d2
Compare
* Random words to add should be 0 Gives very unexpected results, especially weird when validating max char length of a field * Merge with master and minor changes * Update changelog * Minor fix
Gives very unexpected results, especially weird when validating max char length of a field