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

Modify Faker::Educator, Fix #576 #803

Merged

Conversation

ghbooth12
Copy link
Contributor

Hello. This PR is about fixing the issue #576 by adding a method 'subject'. Additionally I have edited the Faker::Educator.course method. Because the existing course method returns a string like "Associate Degree in Criminology". I think this is a name of degree, not a name of course. So I have renamed the course method to the degree method. Then I have created the new course method that returns something like "Criminology 201".

  1. Created subject method that returns something like "Criminology", "Engineering", "Business"...
  2. Renamed course method to degree method
  3. Created course method that returns something like "Criminology 201", "Engineering 411", "Business 157"...

The tests for these changes have been added and all have passed. The documentation(faker/doc/educator.md) has updated for these changes.

@b264
Copy link
Contributor

b264 commented Jan 20, 2017

Maybe we should make Faker::Educator.course an alias for Faker::Educator.degree (except not listed in the docs and with a deprecation warning) and make the new method called Faker::Educator.course_name so we don't break compatibility with existing behaviour

@ghbooth12
Copy link
Contributor Author

Thank you for your feedback @b264. It makes sense to make an alias for course and with a deprecation warning. I updated my code to reflect these changes.

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.

@ghbooth12 could you please update your branch with master and fix the rubocop violations?

@ghbooth12
Copy link
Contributor Author

@vbrazo Absolutely. I'm sorry for my late reply. I'll update this PR this afternoon.

@ghbooth12 ghbooth12 force-pushed the 576-sector-for-universities-and-companies branch from f2fcf57 to 9e67622 Compare July 1, 2018 19:45
@vbrazo vbrazo merged commit 287a190 into faker-ruby:master Aug 20, 2018
davidmorton0 pushed a commit to davidmorton0/faker that referenced this pull request Jul 12, 2021
* Add subject, course_name, degree method for Faker::Educator (Fix faker-ruby#576)

* Update educator.rb

* Update educator.rb
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.

3 participants