Opened 5 years ago

Closed 7 months ago

Last modified 7 months ago

#5645 enhancement closed fixed (fixed)

Document the correct way to deprecate a function in Twisted

Reported by: exarkun Owned by: adiroiban
Priority: normal Milestone:
Component: core Keywords: documentation
Cc: Branch: branches/deprecation-note-list-5645
branch-diff, diff-cov, branch-cov, buildbot
Author: adiroiban

Description

We have a pretty strict process for deprecations. It's simple, but the steps all need to be followed exactly:

  • Do not change the function's behavior as part of this process
  • Cause usage of the function to emit a DeprecationWarning
    • either call warnings.warn
    • or use one of the APIs in twisted.python.deprecate
  • The warning text must include the version of Twisted in which the function is first deprecated (which will always be a version in the future)
  • The warning text should recommend a replacement, if one exists
  • The warning must "point to" the code which called the function. For example, in the normal case, this means stacklevel=2 passed to warnings.warn.
  • There must be a unit test which verifies these things.
    • Unit tests can use TestCase.flushWarnings or callDeprecated or one of the other helpers.
  • A .removal news fragment must be added.

Maybe I forgot some things, too.

Change History (9)

comment:1 Changed 4 years ago by tom.prince

  • Tests of the deprecated function must be left, and appropriate .suppress attribute added to avoid emitting the DeprecationWarning for them.

comment:2 Changed 4 years ago by tom.prince

#6266 is relevant for making remaining tests not emit deprecation warnings.

comment:3 Changed 4 years ago by tom.prince

#3266 is also related.

comment:4 Changed 8 months ago by adiroiban

  • Author set to adiroiban
  • Branch set to branches/deprecation-note-list-5645

(In [47329]) Branching to deprecation-note-list-5645.

comment:5 Changed 8 months ago by adiroiban

  • Keywords review added

I have updated the documentation to include the list from the ticket description.

Please check that it make sense and feel free to commit your suggestions

Thanks!

comment:6 Changed 7 months ago by glyph

  • Owner set to adiroiban

We should probably improve this a bit more, but the doc builder is happy and this is a good first step towards a more expansive to-do guide. Please merge!

comment:7 Changed 7 months ago by glyph

  • Keywords review removed

comment:8 Changed 7 months ago by adiroiban

  • Resolution set to fixed
  • Status changed from new to closed

(In [47340]) Merge deprecation-note-list-5645: To-do guide for deprecating.

Author: adiroiban Reviewer: glyph Fixes: #5645

comment:9 Changed 7 months ago by adiroiban

Thanks for the review.

I assume that when a new developer will try to read the documentation, she can provide valuable feedback and update the list.

I am already a bit familiar with the deprecation process and I find it hard to know what parts are not clear for new developers.

Note: See TracTickets for help on using tickets.