Opened 3 years ago

Closed 20 months ago

#5564 task closed fixed (fixed)

Remove deprecated API in t.names.root

Reported by: thijs Owned by: tom.prince
Priority: normal Milestone:
Component: names Keywords: easy
Cc: thijs Branch: branches/names-root-5564
(diff, github, buildbot, log)
Author: piffey Launchpad Bug:

Description

The following were deprecated in 10.0 (r27740/#970) and can be removed:

  • retry
  • Resolver.discoveredAuthority
  • lookupNameservers
  • lookupAddress
  • extractAuthority
  • discoverAuthority

Attachments (3)

5564.patch (12.5 KB) - added by piffey 3 years ago.
5564.removal (681 bytes) - added by piffey 3 years ago.
Must have forgotten to grab the new file with svn diff. Whoops!
5564.removal-2 (197 bytes) - added by piffey 3 years ago.
Revision to cut down on repetition.

Download all attachments as: .zip

Change History (16)

Changed 3 years ago by piffey

comment:1 follow-up: Changed 3 years ago by piffey

  • Keywords review added

First time contributing, followed the model of other tickets and hope everything is proper. Added news file already as well.

comment:2 Changed 3 years ago by thijs

  • Author set to thijs
  • Branch set to branches/names-root-5564

(In [33962]) Branching to 'names-root-5564'

comment:3 in reply to: ↑ 1 Changed 3 years ago by thijs

  • Author changed from thijs to piffey
  • Keywords review removed
  • Owner set to piffey

Replying to piffey:

First time contributing, followed the model of other tickets and hope everything is proper. Added news file already as well.

Thanks. All looks good but the news file wasn't included in the patch. Can you attach it here?

Changed 3 years ago by piffey

Must have forgotten to grab the new file with svn diff. Whoops!

comment:4 follow-up: Changed 3 years ago by exarkun

Thanks. The news file doesn't need to mention the removed unit tests, only the removed public APIs. Also, if you can factor out the duplicate text in a way that doesn't make it sound awkward, that'd be great.

To future reviewers, though this looks pretty good and is probably just about in shape to merge to trunk, please do not merge to trunk yet.

Changed 3 years ago by piffey

Revision to cut down on repetition.

comment:5 Changed 3 years ago by itamar

  • Keywords review added

comment:6 in reply to: ↑ 4 Changed 3 years ago by thijs

  • Owner changed from piffey to exarkun

Replying to exarkun:

To future reviewers, though this looks pretty good and is probably just about in shape to merge to trunk, please do not merge to trunk yet.

I added the news file in r34122. Assigning to exarkun for review based on his comment 4.

comment:7 follow-up: Changed 3 years ago by exarkun

  • Keywords review removed

Don't think there's anything left to review here.

comment:8 in reply to: ↑ 7 Changed 2 years ago by thijs

  • Owner changed from exarkun to thijs
  • Status changed from new to assigned

Replying to exarkun:

Don't think there's anything left to review here.

Alright, I'll merge it then.

comment:9 follow-up: Changed 2 years ago by exarkun

  • Owner changed from thijs to exarkun
  • Status changed from assigned to new

Please don't. There's no reason to rush to remove deprecated things. We should leave them *as long as possible*.

comment:10 in reply to: ↑ 9 Changed 2 years ago by thijs

Replying to exarkun:

Please don't. There's no reason to rush to remove deprecated things. We should leave them *as long as possible*.

Thanks for the clarification, comment 7 got me confused.

comment:11 Changed 20 months ago by tom.prince

I'm working on #6044, and so need to change all uses of deferredGenerator. discoverAuthority uses it, so this ticket can probably land now.

comment:12 Changed 20 months ago by exarkun

  • Owner changed from exarkun to tom.prince

That's reasonable.

comment:13 Changed 20 months ago by tomprince

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

(In [37287]) Merge names-root-5564: Remove deprecated API in twisted.names.root.

Author: piffey
Reviewers: thijs
Fixes: #5564

The following were deprecated in 10.0 (r27740/#970) and can be removed:

  • retry
  • Resolver.discoveredAuthority
  • lookupNameservers
  • lookupAddress
  • extractAuthority
  • discoverAuthority
Note: See TracTickets for help on using tickets.