-
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
Added Generator for International Phonetic Alphabet Characters #2777
Conversation
…n the international phonetic alphabet
…aracter would be generated, and added tests for generating multiple characters
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.
Few comments related to code quality. Also, please fix formatting in PR description. Thanks.
# | ||
# @faker.version next | ||
def vowel(amount: 1) | ||
(1..amount).map{fetch('international_phonetic_alphabet.vowels')}.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.
Please fix formatting here.
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.
Rubocop have an autocorrect method: rubocop -a $file_path
.
- a | ||
- ɶ | ||
- ɑ | ||
- ɒ |
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.
Please add newline at the end of the file.
assert_match(/..../, @tester.character(amount: 4)) | ||
end | ||
|
||
end |
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.
Please add newline at the end of the file.
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.
Thank you for this! Besides to what @jremes-foss has mentioned, I left some suggestions for this PR. Thanks!
module Faker | ||
class InternationalPhoneticAlphabet < Base | ||
|
||
class << self |
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.
What do you think of putting a limit for the amount? Without a limit here, the user can pass a really long number to the methods. An idea is to add a limit and raise an argument error if the user asks for an amount bigger than that. 1000 seems like a reasonable amount.
# | ||
# @faker.version next | ||
def vowel(amount: 1) | ||
(1..amount).map{fetch('international_phonetic_alphabet.vowels')}.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.
Rubocop have an autocorrect method: rubocop -a $file_path
.
|
||
class TestFakerInternationalPhoneticAlphabet < Test::Unit::TestCase | ||
def setup | ||
@tester = Faker::InternationalPhoneticAlphabet |
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.
Could you add a test for when amount
is 0 and a negative number?
hi @Grifff17 thank you for your contribution. In an effort to keep our work manageable, PRs that haven't had any activity for the past month will be closed. Feel free to reopen it when you're ready and address the comments here. Thanks! |
Motivation / Background
This Pull Request has been created because I have added a new generator that generates characters in the International Phonetic Alphabet. It can generate vowels, consonants, or just general characters. I have followed the documentations guidelines.
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
If you're proposing a new generator: