Page MenuHomePhabricator

bundlerepo: make methods agree with base class
ClosedPublic

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

Details

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
dlax added a subscriber: dlax.Nov 12 2017, 3:10 AM

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#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
This revision was automatically updated to reflect the committed changes.