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

Random words to add should be 0 #719

Merged

Conversation

swapsCAPS
Copy link
Contributor

Gives very unexpected results, especially weird when validating max char length of a field

Gives very unexpected results, especially weird when validating max char length of a field
@vbrazo vbrazo force-pushed the master branch 5 times, most recently from a359def to a5d7731 Compare May 22, 2018 21:15
Copy link
Member

@vbrazo vbrazo left a 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.

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 + '.'
Copy link
Member

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

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 + '?'
Copy link
Member

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

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(' ')
Copy link
Member

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 👍

@vbrazo vbrazo force-pushed the patch-default-random-sentence-length branch from ad7704c to dc4d3d2 Compare June 11, 2018 02:11
@vbrazo vbrazo merged commit b1d6bd0 into faker-ruby:master Jun 11, 2018
@vbrazo vbrazo self-requested a review July 19, 2018 01:39
davidmorton0 pushed a commit to davidmorton0/faker that referenced this pull request Jul 12, 2021
* 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
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.

2 participants