-
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
Fixing documentation - Faker::Name to Faker::Zelda #1256
Fixing documentation - Faker::Name to Faker::Zelda #1256
Conversation
Amazing dude |
These |
@vbrazo Yeah, I thought that might be the case. I had the same impression for the |
@vbrazo there appear to be tests for those additional Maybe this could be another PR where we remove those methods, because they only seem to be used in conjunction with the |
I think we shouldn't modify them for now. Let's fix the issues or add the missing tests/docs first and then we could think about improvements like this one in the future @mrstebo |
Also looking at |
…ssing through to method_missing.
@@ -1,6 +1,6 @@ | |||
# Faker::Hobbit | |||
# Faker::NatoPhoneticAlphabet |
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.
copy and 🍝 😆
|
||
```ruby | ||
# A code word from the NATO phonetic alphabet | ||
Faker::NatoPhoneticAlphabet #=> "Hotel" | ||
Faker::NatoPhoneticAlphabet.code_word #=> "Hotel" |
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.
👍
def sport | ||
fetch('team.sport') | ||
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.
👍
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.
Looks pretty good. Thanks 🥇
@@ -17,4 +17,6 @@ Faker::Name.title #=> "Legacy Creative Director" | |||
|
|||
Faker::Name.initials #=> "NJM" | |||
Faker::Name.initials(2) #=> "NM" | |||
|
|||
Faker::Name.job_titles #=> [""Supervisor", "Associate", ...] |
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.
I think we should remove this method because now we have a Faker::Job
API @mrstebo
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.
Do you want to open a PR and add a description explaining what's going on and fix it? We need to remove the locales, docs, tests and the method.
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.
Sure, no problem 👍
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.
When a method that has been in a gem release is being removed, we need to deprecate it before removing it so we don’t break people’s code. This would including printing a warning message in the release before it will be removed.
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.
@mrstebo your PR would need to wait for 2 versions
* Added missing method to name documentation. * Removed duplicate capital_city documentation. * Fixed title and added code_word method to Faker::NatoPhoneticAlphabet documentation. * Added missing methods to the Faker::Number documentation. * Added call_squadron and call_number to Faker::StarWars documentation. * Added explicit sport method in Faker::Team. Previously it was just passing through to method_missing. * Added prefix, suffix and greek_alphabet to Faker::University documentation.
An extension of #1252.
Here are the issues that I have found relating to the documentation.
Faker::Name.job_titles
- not in docsFaker::Nation.capital_city
- duplicated in docsFaker::NatoPhoneticAlphabet
- incorrect title in docFaker::NatoPhoneticAlphabet.code_word
- missing method name in docFaker::Number.leading_zero_number
- not in docsFaker::Number.decimal_part
- not in docsFaker::Number.non_zero_digit
- not in docsFaker::StarWars.call_squadron
- not in docsFaker::StarWars.call_number
- not in docsFaker::Team.sport
- no codeFaker::University.prefix
- not in docs (should be protected?)Faker::University.suffix
- not in docs (should be protected?)Faker::University.greek_alphabet
- not in docs (should be private?)