( )⚙ D2723 httppeer: consolidate _requestbuilder assignments and document

This is an archive of the discontinued Mercurial Phabricator instance.

httppeer: consolidate _requestbuilder assignments and document
ClosedPublic

Authored by indygreg on Mar 8 2018, 12:31 AM.

Details

Summary

I collapsed multiple assignments because they don't appear to
be necessary. We don't invoke the requestbuilder in anything
called during init. So there's no reason to not define it
earlier AFAICT.

This seemingly useless attribute is actually an extension point.
Document it as such.

(A previous version of this patch removed the attribute because
it appeared to just be an alias.)

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Mar 8 2018, 12:31 AM
indygreg updated this revision to Diff 6973.Mar 12 2018, 8:07 PM
pulkit added a subscriber: pulkit.Mar 15 2018, 12:18 PM

Looks like this was added so that extensions can replace using urllib2.Request. https://www.mercurial-scm.org/repo/hg/rev/00ecc894138d

In D2723#46187, @pulkit wrote:

Looks like this was added so that extensions can replace using urllib2.Request. https://www.mercurial-scm.org/repo/hg/rev/00ecc894138d

Thanks for looking that up. We still depend on it, so we'd appreciate if it could stay. Perhaps we should have added comment saying that it was there for extensibility.

indygreg edited the summary of this revision. (Show Details)Mar 15 2018, 2:20 PM
indygreg retitled this revision from httppeer: remove _requestbuilder attribute to httppeer: consolidate _requestbuilder assignments and document.
indygreg updated this revision to Diff 7070.
pulkit accepted this revision.Mar 15 2018, 2:24 PM
This revision was automatically updated to reflect the committed changes.