This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1372

bundlerepo: make methods agree with base class
ClosedPublic

Authored by indygreg on Nov 11 2017, 9:51 PM.

Download Raw Diff

Details

Reviewers

durin42

Group Reviewers

hg-reviewers

Commits

rHG32d079f37207: bundlerepo: make methods agree with base class

Summary

My editor was complaining about mismatches between method
signatures.

For methods that are implemented, we change arguments to match
the base. For those that aren't, we use variable arguments
because it shouldn't matter.

Diff Detail

Repository

rHG Mercurial

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Nov 11 2017, 9:51 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptNov 11 2017, 9:51 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

indygreg added a child revision: D1373: bundlerepo: use suffix variable.Nov 11 2017, 9:51 PM

For methods that are implemented, we change arguments to match the base.

Alternatively, we could use **kwargs for keywords arguments unused in a method. I think that's a common pattern and it avoids confusing the reader with unused arguments.

In D1372#22814, @dlax wrote:

For methods that are implemented, we change arguments to match the base.

Alternatively, we could use **kwargs for keywords arguments unused in a method. I think that's a common pattern and it avoids confusing the reader with unused arguments.

👍 for using **kwargs and even maybe *args if necessary.

In D1372#22818, @lothiraldan wrote:

In D1372#22814, @dlax wrote:

For methods that are implemented, we change arguments to match the base.

Alternatively, we could use **kwargs for keywords arguments unused in a method. I think that's a common pattern and it avoids confusing the reader with unused arguments.

👍 for using **kwargs and even maybe *args if necessary.

The standard we've recently moved to at Google is to allow del foo # unused for unused arguments, so they're forcibly unused, but also still demonstrably passed. It makes things easier overall for various inference tools, and I'd be happy to adopt that in hg too.

durin42 accepted this revision.Nov 13 2017, 5:15 PM

This revision is now accepted and ready to land.Nov 13 2017, 5:15 PM

Closed by commit rHG32d079f37207: bundlerepo: make methods agree with base class (authored by indygreg). · Explain WhyNov 13 2017, 6:16 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/bundlerepo.py (15 lines)

Status	Author	Revision
Closed	indygreg	D1379 bundlerepo: rename _bundle to _cgunpacker
Closed	indygreg	D1378 bundlerepo: assign bundle attributes in bundle type blocks
Closed	indygreg	D1377 bundlerepo: make bundle and bundlefile attributes private
Closed	indygreg	D1376 bundlerepo: don't assume there are only two bundle classes
Closed	indygreg	D1375 bundlerepo: add docstring for bundlerepository class
Closed	indygreg	D1374 bundlerepo: rename arguments to bundlerepository.__init__
Closed	indygreg	D1373 bundlerepo: use suffix variable
Closed	indygreg	D1372 bundlerepo: make methods agree with base class

Diff 3449

mercurial/bundlerepo.py

	# start, size, full unc. size, base (unused), link, p1, p2, node			# start, size, full unc. size, base (unused), link, p1, p2, node
	e = (revlog.offset_type(start, flags), size, -1, baserev, link,			e = (revlog.offset_type(start, flags), size, -1, baserev, link,
	self.rev(p1), self.rev(p2), node)			self.rev(p1), self.rev(p2), node)
	self.index.insert(-1, e)			self.index.insert(-1, e)
	self.nodemap[node] = n			self.nodemap[node] = n
	self.bundlerevs.add(n)			self.bundlerevs.add(n)
	n += 1			n += 1

	def _chunk(self, rev):			def _chunk(self, rev, df=None):
	# Warning: in case of bundle, the diff is against what we stored as			# Warning: in case of bundle, the diff is against what we stored as
	# delta base, not against rev - 1			# delta base, not against rev - 1
	# XXX: could use some caching			# XXX: could use some caching
	if rev <= self.repotiprev:			if rev <= self.repotiprev:
	return revlog.revlog._chunk(self, rev)			return revlog.revlog._chunk(self, rev)
	self.bundle.seek(self.start(rev))			self.bundle.seek(self.start(rev))
	return self.bundle.read(self.length(rev))			return self.bundle.read(self.length(rev))

	def revdiff(self, rev1, rev2):			def revdiff(self, rev1, rev2):
	"""return or calculate a delta between two revisions"""			"""return or calculate a delta between two revisions"""
	if rev1 > self.repotiprev and rev2 > self.repotiprev:			if rev1 > self.repotiprev and rev2 > self.repotiprev:
	# hot path for bundle			# hot path for bundle
	revb = self.index[rev2][3]			revb = self.index[rev2][3]
	if revb == rev1:			if revb == rev1:
	return self._chunk(rev2)			return self._chunk(rev2)
	elif rev1 <= self.repotiprev and rev2 <= self.repotiprev:			elif rev1 <= self.repotiprev and rev2 <= self.repotiprev:
	return revlog.revlog.revdiff(self, rev1, rev2)			return revlog.revlog.revdiff(self, rev1, rev2)

	return mdiff.textdiff(self.revision(rev1, raw=True),			return mdiff.textdiff(self.revision(rev1, raw=True),
	self.revision(rev2, raw=True))			self.revision(rev2, raw=True))

	def revision(self, nodeorrev, raw=False):			def revision(self, nodeorrev, _df=None, raw=False):
	"""return an uncompressed revision of a given node or revision			"""return an uncompressed revision of a given node or revision
	number.			number.
	"""			"""
	if isinstance(nodeorrev, int):			if isinstance(nodeorrev, int):
	rev = nodeorrev			rev = nodeorrev
	node = self.node(rev)			node = self.node(rev)
	else:			else:
	node = nodeorrev			node = nodeorrev
	return text			return text

	def baserevision(self, nodeorrev):			def baserevision(self, nodeorrev):
	# Revlog subclasses may override 'revision' method to modify format of			# Revlog subclasses may override 'revision' method to modify format of
	# content retrieved from revlog. To use bundlerevlog with such class one			# content retrieved from revlog. To use bundlerevlog with such class one
	# needs to override 'baserevision' and make more specific call here.			# needs to override 'baserevision' and make more specific call here.
	return revlog.revlog.revision(self, nodeorrev, raw=True)			return revlog.revlog.revision(self, nodeorrev, raw=True)

	def addrevision(self, text, transaction, link, p1=None, p2=None, d=None):			def addrevision(self, args, *kwargs):
	raise NotImplementedError			raise NotImplementedError
	def addgroup(self, deltas, transaction, addrevisioncb=None):
				def addgroup(self, args, *kwargs):
	raise NotImplementedError			raise NotImplementedError
	def strip(self, rev, minlink):
				def strip(self, args, *kwargs):
	raise NotImplementedError			raise NotImplementedError

	def checksize(self):			def checksize(self):
	raise NotImplementedError			raise NotImplementedError

	class bundlechangelog(bundlerevlog, changelog.changelog):			class bundlechangelog(bundlerevlog, changelog.changelog):
	def __init__(self, opener, bundle):			def __init__(self, opener, bundle):
	changelog.changelog.__init__(self, opener)			changelog.changelog.__init__(self, opener)
	linkmapper = lambda x: x			linkmapper = lambda x: x
	bundlerevlog.__init__(self, opener, self.indexfile, bundle,			bundlerevlog.__init__(self, opener, self.indexfile, bundle,

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	3427		Nov 11 2017, 9:51 PM	★	★
Diff 2	3449	rHG32d079f372078defdd10ad28a118f3cb15082e0e	Nov 11 2017, 8:02 PM	★	★