Opened 6 years ago

Closed 5 years ago

#5564 task closed fixed (fixed)

Remove deprecated API in t.names.root

Reported by: Thijs Triemstra Owned by: Tom Prince
Priority: normal Milestone:
Component: names Keywords: easy
Cc: Thijs Triemstra Branch: branches/names-root-5564
branch-diff, diff-cov, branch-cov, buildbot
Author: piffey

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 6 years ago.
5564.removal​ (681 bytes) - added by Piffey 6 years ago.
Must have forgotten to grab the new file with svn diff. Whoops!
5564.removal-2​ (197 bytes) - added by Piffey 6 years ago.
Revision to cut down on repetition.

Download all attachments as: .zip

Change History (16)

Changed 6 years ago by Piffey

Attachment: 5564.patch​ added

comment:1 Changed 6 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 6 years ago by Thijs Triemstra

Author: → thijs
Branch: → branches/names-root-5564

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

comment:3 in reply to:  1 Changed 6 years ago by Thijs Triemstra

Author: thijs → 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 6 years ago by Piffey

Attachment: 5564.removal​ added

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

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

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 6 years ago by Piffey

Attachment: 5564.removal-2​ added

Revision to cut down on repetition.

comment:5 Changed 6 years ago by Itamar Turner-Trauring

Keywords: review added

comment:6 in reply to:  4 Changed 6 years ago by Thijs Triemstra

Owner: changed from Piffey to Jean-Paul Calderone

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 Changed 6 years ago by Jean-Paul Calderone

Keywords: review removed

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

comment:8 in reply to:  7 Changed 5 years ago by Thijs Triemstra

Owner: changed from Jean-Paul Calderone to Thijs Triemstra
Status: new → assigned

Replying to exarkun:

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

Alright, I'll merge it then.

comment:9 Changed 5 years ago by Jean-Paul Calderone

Owner: changed from Thijs Triemstra to Jean-Paul Calderone
Status: assigned → 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 5 years ago by Thijs Triemstra

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 5 years 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 5 years ago by Jean-Paul Calderone

Owner: changed from Jean-Paul Calderone to Tom Prince

That's reasonable.

comment:13 Changed 5 years ago by Tom Prince

Resolution: → fixed
Status: new → 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.