Page MenuHomePhabricator

commands: necessary annotations and suppresssions to pass pytype
Needs ReviewPublic

Authored by durin42 on Nov 13 2019, 10:51 PM.

Details

Reviewers
dlax
marmoute
Group Reviewers
hg-reviewers
Summary

As with other places, there are some places where our types are just
too complicated for pytype, so we put some suppressions in place.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durin42 created this revision.Nov 13 2019, 10:51 PM
dlax added a subscriber: dlax.Nov 14 2019, 4:45 AM
dlax added inline comments.
mercurial/commands.py
1124

This one is sad. I think this can be sorted out by replacing the try:/finally: with a context manager. (Can send a patch, if it sounds good to you.)

durin42 added inline comments.Nov 14 2019, 4:15 PM
mercurial/commands.py
1124

By all means!

dlax requested changes to this revision.Nov 15 2019, 6:08 AM

D7430 makes this changes unnecessary I think.

This revision now requires changes to proceed.Nov 15 2019, 6:08 AM
durin42 marked an inline comment as done.Nov 15 2019, 12:23 PM

I still want to keep the annotations I added. :)

durin42 updated this revision to Diff 18162.Nov 15 2019, 12:31 PM
dlax added inline comments.Nov 15 2019, 12:40 PM
mercurial/commands.py
4746

revs is always a smartset.baseset per af9c73f26371 so there should be no attribute error. Or is it because logcmdutil.getlinerangerevs() has no type annotation (whereas logcmdutil.getrevs() has some)?

durin42 updated this revision to Diff 18171.Nov 15 2019, 4:55 PM

What's up on this ? It seemed on a good track, but I don't think it landed. @dlax I think you offer to use a context manager got a warm welcome, I would says, go ahead with it.

dlax added a comment.Thu, Jan 30, 2:26 PM

What's up on this ? It seemed on a good track, but I don't think it landed. @dlax I think you offer to use a context manager got a warm welcome, I would says, go ahead with it.

The context manager got in through D7430.

mercurial/commands.py
4741

I think this comment should be removed since the # pytype: disable got removed in the last version of the diff.

marmoute accepted this revision.Fri, Feb 14, 3:21 AM

There don't seems to be any remaininf feedback on this.