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

Added Faker::Blood #1960

Merged
merged 3 commits into from
May 19, 2020
Merged

Conversation

surajput32
Copy link
Contributor

@surajput32 surajput32 commented Apr 5, 2020

Summary:

Added a new class Faker::Blood with the following methods:

  • Faker::Blood.type #=> AB
  • Faker::Blood.rh_factor #=> -
  • Faker::Blood.group #=> AB-

Issue#

No-Story

Description:

In my company, there are medical related projects where we store patients data(i.e. Blood group).
In this PR, I added a new class Faker::Blood with some important methods. I hope this will be useful in similar projects.

@lucasqueiroz
Copy link
Member

Hi there, thanks for the Pull Request!
A suggestion I have would be creating a method for the blood type (A, B, AB and O) and one for the Rh Factor (+, -), and maybe one where both could be interpolated.
Let me know what you think :)

Thanks

@surajput32
Copy link
Contributor Author

Hi there, thanks for the Pull Request!
A suggestion I have would be creating a method for the blood type (A, B, AB and O) and one for the Rh Factor (+, -), and maybe one where both could be interpolated.
Let me know what you think :)

Thanks

Hi @lucasqueiroz, I agree with your suggestion and will do the changes soon. Please confirm the methods and their names.

  1. Faker::Blood.type #=> AB
  2. Faker::Blood.rh_factor #=> -
  3. Faker::Blood.group #=> AB-

Let me know if any changes are required.

@lucasqueiroz
Copy link
Member

Sounds good to me!
Would like to know what other contributors think as well :)
@vbrazo @stympy could you guys validate this?
Thanks

@the-spectator
Copy link

@suraj32 The proposed API really looks good!

@surajput32
Copy link
Contributor Author

Sounds good to me!
Would like to know what other contributors think as well :)
@vbrazo @stympy could you guys validate this?
Thanks

Hi @lucasqueiroz . As discussed, I added new methods & updated the description. Let me know if there are any issues with this.

Copy link

@the-spectator the-spectator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@suraj32 We can improve the test cases.

test/faker/default/test_faker_blood.rb Outdated Show resolved Hide resolved
test/faker/default/test_faker_blood.rb Outdated Show resolved Hide resolved
test/faker/default/test_faker_blood.rb Outdated Show resolved Hide resolved
@surajput32 surajput32 changed the title Added Faker::Blood.group Added Faker::Blood May 5, 2020
@Zeragamba Zeragamba merged commit ea5a9f1 into faker-ruby:master May 19, 2020
@Zeragamba
Copy link
Contributor

Thanks!

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.

5 participants