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

Prepare Faker::Name.job_titles and Faker::Name.title for deprecation #1264

Merged

Conversation

mrstebo
Copy link
Contributor

@mrstebo mrstebo commented May 31, 2018

As mentioned in #1256 we should not take away the job-like methods from Faker::Name straight away. Instead we should add deprecation warnings.

Could you offer some advise on how we are notifying consumers of the deprecation, and then I'll make the appropriate changes?

  • Add deprecation warnings
  • Remove methods from the Faker::Name documentation?

@vbrazo
Copy link
Member

vbrazo commented May 31, 2018

@mrstebo when you update the changelog, please make sure to create a new group Depreciation and add your title with the basic info for our internal control. It'll help us when we release a new version.

@mrstebo
Copy link
Contributor Author

mrstebo commented Jun 1, 2018

@vbrazo Done! Everything look okay?

@gkunwar
Copy link
Contributor

gkunwar commented Jun 7, 2018

@mrstebo @vbrazo it looks good for me.

@vbrazo
Copy link
Member

vbrazo commented Jun 9, 2018

@stympy how do we display the deprecation message in the Faker gem?

@stympy
Copy link
Contributor

stympy commented Jun 10, 2018

I suppose we could use Gem::Deprecate. The year and month should be September 2018.

@mrstebo
Copy link
Contributor Author

mrstebo commented Jun 10, 2018

@stympy @vbrazo I added Gem::Deprecate and it all works fine. But when we run the tests we get a bunch of warnings about the deprecated methods.

Should these be left in so that we know what needs to change before the methods are removed? Or do we add in a before hook into the tests to suppress these warnings?

Personally, I'd prefer to keep these warnings in there. Gives a bigger incentive for someone to come along and fix the warnings 🤔

@vbrazo vbrazo force-pushed the refactor/prepare-job-titles-for-deprecation branch from 733adbe to 78c316f Compare June 11, 2018 00:54
@vbrazo vbrazo closed this Jun 11, 2018
@vbrazo vbrazo reopened this Jun 11, 2018
@vbrazo vbrazo merged commit acaa80e into faker-ruby:master Jun 11, 2018
@mrstebo mrstebo deleted the refactor/prepare-job-titles-for-deprecation branch June 11, 2018 14:00
@vbrazo vbrazo self-requested a review July 19, 2018 01:39
davidmorton0 pushed a commit to davidmorton0/faker that referenced this pull request Jul 12, 2021
…aker-ruby#1264)

* Added additional levels of seniority.

* Using Faker::Job to return a job title.

* Making job_titles fetch the list of jobs from the jobs data file.

* Removed the title section from the name data file.

* Removed methods from Faker::Name documentation.

* Added Deprecation section to the changelog with a link to the PR.

* Added Gem::Deprecate to show warnings of deprecated methods.

* Remove Rubygems deprecation warnings
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.

4 participants