Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#7828 enhancement closed fixed (fixed)

Remove deprecated functions from t.web.util

Reported by: hawkowl Owned by: hawkowl
Priority: normal Milestone:
Component: web Keywords:
Cc: Branch: branches/twutil-rmdeprecations-7828
branch-diff, diff-cov, branch-cov, buildbot
Author: hawkowl

Description

Since I need to port this module, I noticed there was a bunch of stuff that's been deprecated since 2012 and is awful stuff that nobody should use.

Let's remove it!

Change History (6)

comment:1 Changed 7 years ago by hawkowl

Author: hawkowl
Branch: branches/twutil-rmdeprecations-7828

(In [44208]) Branching to twutil-rmdeprecations-7828.

comment:2 Changed 7 years ago by hawkowl

Keywords: review added

Removed them, pushed up, builders spun.

Please review.

comment:3 Changed 7 years ago by Chris Wolfe

Keywords: review removed
Owner: set to hawkowl

Thanks for your work!

  1. It looks like this complies with the Twisted deprecation policy detailed at http://twistedmatrix.com/trac/wiki/CompatibilityPolicy#IncompatibleChanges.
    1. Your patch removes deprecated code three major versions after the deprecation warnings were added (added in 12.1). The policy requires that the warnings be present for at least 2 releases, which they has been.
    2. Your patch removes deprecated code ~2.5/3 years after deprecation (12.1's release date was June 4, 2012). The policy requires that the deprecated code be present with warnings for at least one year, which it certainly has been.
  2. Code:
    1. All of the builders are green.
    2. You've done a bit of extra cleanup by adding the __all__ to twisted.web.util, nice.
    3. It looks like there are no instances of the deprecated functions/variables lurking around elsewhere in the source. I verified this using git grep on the following:
      htmlFunc
      htmlInst
      htmlReprTypes
      htmlList
      htmlDict
      htmlUnknown
      saferepr
      htmlrepr
      

I was unable to find any instances of twisted.web.util.stylesheet, but it could have gotten past my search.

The patch looks good to me. Please land it if you feel my review is sufficient!

Thanks again!

comment:4 Changed 7 years ago by hawkowl

Hi herrwolfe!

Thanks for the review!

The all was actually at the top, I just moved it to the bottom while I was changing it :)

Will merge!

-Hawkie

comment:5 Changed 7 years ago by hawkowl

Resolution: fixed
Status: newclosed

(In [44259]) Merging twutil-rmdeprecations-7828: Remove deprecated functions from t.web.util

Author: hawkowl Reviewer: herrwolfe Fixes: #7828

comment:6 Changed 7 years ago by Jean-Paul Calderone

Why did you move __all__?

Note: See TracTickets for help on using tickets.