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

Introduce RemovedInDRF…Warning classes to simplify deprecations. #6480

Merged
merged 1 commit into from Mar 3, 2019

Conversation

Projects
None yet
4 participants
@carltongibson
Copy link
Collaborator

carltongibson commented Mar 1, 2019

Closes #6290.

@carltongibson carltongibson added this to the 3.9.2 Release milestone Mar 1, 2019

@carltongibson carltongibson requested review from tomchristie and rpkilby Mar 1, 2019

@carltongibson

This comment has been minimized.

Copy link
Collaborator Author

carltongibson commented Mar 1, 2019

@tomchristie @rpkilby I quite like this. It adds a small amount of code by makes handling deprecations a lot simpler.

  • Branch for 3.10, delete RemovedInDRF310Warning, delete all the code that breaks.
    • Note that for release announcement.
  • Bump RemovedInDRF311Warning to DeprecationWarning.
    • Note for release announcement.

Much harder to miss anything.

Let me know what you think. If we're happy with it I'll merge.

Either way, this wraps up 3.9.2 🎉 I'll prepare the release over the weekend. 💃

@rpkilby

This comment has been minimized.

Copy link
Member

rpkilby commented Mar 1, 2019

I'm definitely in favor of this.

@auvipy

auvipy approved these changes Mar 2, 2019

@tomchristie

This comment has been minimized.

Copy link
Member

tomchristie commented Mar 2, 2019

@auvipy I think I've asked this before, but please don't use the PR approval unless you're actively involved in a ticket. (As it happens I'd rather it was only enabled for maintainers or by-request-from-maintainers, but GitHub doesn't allow us to turn the functionality off.)

@tomchristie
Copy link
Member

tomchristie left a comment

Good call. Thanks @carltongibson!

@auvipy

This comment has been minimized.

Copy link
Contributor

auvipy commented Mar 3, 2019

@tomchristie I don't use PR approve button unless I go through it fully. I don't know how you manage to do this in an open source project? respectfully disagree with you on this point.

@carltongibson

This comment has been minimized.

Copy link
Collaborator Author

carltongibson commented Mar 3, 2019

Hi @auvipy.

To be honest I too find it distracting that you approve PRs without adding to the conversation.

For me it would be better if you spent your time looking at less PRs but reviewed them fully, with comments and improvements etc. There’s a massive backlog around the ecosystem so plenty to do there.

Then if there were some PRs that you just approved that would tell something. As it stands it your tick isn’t often acting as a marker, since it’s too quickly given, so it’s not serving as a guide as to whether to merge.

I hope that makes sense. It’s meant in a spirit of love and encouragement. Please think about it. Happy to chat off thread if you like. C.

@carltongibson carltongibson merged commit 94593b3 into encode:master Mar 3, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@carltongibson carltongibson deleted the carltongibson:simplify-deprecations branch Mar 3, 2019

@auvipy

This comment has been minimized.

Copy link
Contributor

auvipy commented Mar 3, 2019

@carltongibson I would really love to engage that way. I get paid for working with foss. so why not :)

@carltongibson

This comment has been minimized.

Copy link
Collaborator Author

carltongibson commented Mar 3, 2019

@auvipy OK, send me an email. 👍

@auvipy

This comment has been minimized.

Copy link
Contributor

auvipy commented Mar 3, 2019

definitely, I will. you just won my heart by your words Carlton <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.