Closed stefannibrasil closed 1 year ago
@thdaraujo could you update the tag to be hacktoberfest-accepted
? Seems to be the new requirement from the event https://hacktoberfest.com/participation/#pr-mr-details
@thdaraujo could you update the tag to be
hacktoberfest-accepted
? Seems to be the new requirement from the event https://hacktoberfest.com/participation/#pr-mr-details
hey @mauromorales, thanks for submitting a PR!
Correct me if I'm wrong, but the way I understand it, the issue should have a tag hacktoberfest
, and the accepted pull requests will be tagged as hacktoberfest-accepted
.
Here's what they say about issues:
Apply the “hacktoberfest” label to issues you want contributors to help with in your GitHub or GitLab project.
And about PRs:
Your PR/MRs must be in a repo tagged with the “hacktoberfest” topic, or be labeled “hacktoberfest-accepted”
So when the PR is accepted, we will tag the PR as hacktoberfest-accepted
.
hey @mauromorales, thanks for submitting a PR!
My pleasure, thank you guys for making it easy to spot :)
Correct me if I'm wrong, but the way I understand it, the issue should have a tag
hacktoberfest
, and the accepted pull requests will be tagged ashacktoberfest-accepted
.
Good question, you probably know better than me, the reason why I thought it was the other way around is because of how it looks on the hacktoberfest profile where it's labeled as not-participating
but tbh is not a big deal, we can use this one as a guinea pig to find out if this is how it works or not.
hey @mauromorales, thanks for submitting a PR!
My pleasure, thank you guys for making it easy to spot :)
Correct me if I'm wrong, but the way I understand it, the issue should have a tag
hacktoberfest
, and the accepted pull requests will be tagged ashacktoberfest-accepted
.Good question, you probably know better than me, the reason why I thought it was the other way around is because of how it looks on the hacktoberfest profile where it's labeled as
not-participating
but tbh is not a big deal, we can use this one as a guinea pig to find out if this is how it works or not.![]()
oh I see, that's probably because we did not add the topic yet to the repository, but we should have it soon! :)
And it's been a couple of years since these deprecation warnings were introduced. It's safe to remove them.
JFYI, Not only the migration period, so semver is also important. https://semver.org
It should avoid making changes that could break client software on bundle update
if < 3.0
specified in Gemfile or gemspec .
We saw a lot of negative voices in Faker 2.0 for this change. Let's do our best not to repeat it. So, this task is certainly easy, but it would be preferable whether the incompatible changes were done by maintainer.
That aside, it would have been a catalyst for moving forward to Faker 3.0. Thank you!
hi @koic thank you for the help! I agree it's our job as maintainers to communicate breaking changes in a way that helps users.
The reasoning behind believing that "it's been a couple of years since these deprecation warnings were introduced. It's safe to remove them." was because the keywords arguments were added before the deprecation, and a long time ago.
So reviewing the semantic version docs, I believed that since the deprecation messages were there for a long time and for a breaking change prior to it, it would be safe to remove them and fix the deprecation. I debated for a while if this was or not something major because of that. My bad, I should have raised this confusion in the issue.
To be honest, I was a bit confused by this due to the order of the deprecation. Is there something that I could have done ahead of time to make this a smooth transition, besides mentioning it as a major change in the issue? I like to learn from my mistakes, and I appreciate your support on this!
That said, I agree with making a release for Faker 3.0. @vbrazo are you available for making this release? :blush:
Version 2 of Faker replaced positional arguments with keyword arguments. To reduce users pain and make upgrades easier, deprecation warnings with instructions to upgrade were added.
And it's been a couple of years since these deprecation warnings were introduced. It's safe to remove them.
Here is what needs to be done:
legacy_
arguments, and remove the warnings from them as well. A lot of new methods were added after that PR was merged.