-
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
Add Faker::Military #1283
Add Faker::Military #1283
Conversation
doc/military.md
Outdated
|
||
Faker::Military.us_air_force_rank #=> "Captain" | ||
|
||
Faker::Military.us_dod_paygrade #=> "E-6" |
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'd recommend dropping the us_
because it would help us in our future locale contributions. I mean if we decide to move on with these methods, we'd need to create a chilean_army_rank
, brazilian_army_rank
, spanish_army_rank
and so on. I'd prefer to have only army_rank
+ the locales behind the scenes.
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.
ah cool, yeah my thought was that we could have the chilean_army_rank
in the en.yml
. No worries I'll make the changes now.
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.
We have a few methods in other objects that are named as nationality_
but they involve algorithms to calculate ID numbers or SSN for example. Those methods are a bit particular, that's why we categorize and isolate them. In your case I think we just need the translations. Just like Faker::Name.first_name
.
@@ -0,0 +1,27 @@ | |||
require File.expand_path(File.dirname(__FILE__) + '/test_helper.rb') | |||
|
|||
class TestFakerMilitary < Test::Unit::TestCase |
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.
✊
@@ -0,0 +1,13 @@ | |||
# Faker::Military | |||
|
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 please add something like It might be available in the next version
just like I did 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.
done.
lib/faker/military.rb
Outdated
|
||
def us_dod_paygrade | ||
fetch('military.us_dod_paygrade') | ||
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.
Travis is crashing because you still need to rename a few things in this file @jjasghar
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.
opps! Fixing.
Added US military ranks for Army, Navy, Air Force, and Marines. Also the DOD Pay scale. Signed-off-by: JJ Asghar <jj@chef.io>
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.
Thanks for contributing 💯 and welcome!
* Added US Military ranks Added US military ranks for Army, Navy, Air Force, and Marines. Also the DOD Pay scale. Signed-off-by: JJ Asghar <jj@chef.io> * Update CHANGELOG.md
Added US military ranks for Army, Navy, Air Force, and Marines. Also the
DOD Pay scale.
Signed-off-by: JJ Asghar jj@chef.io