Page MenuHomePhabricator

pytype: stop excluding changegroup.py
ClosedPublic

Authored by mharbison72 on Dec 14 2021, 4:10 PM.

Details

Summary

The false positives that were detected seem to be related to what happens to the
variables after the local methods are used:

File "/mnt/c/Users/Matt/hg/mercurial/changegroup.py", line 353, in ondupchangelog:
    No attribute 'append' on range [attribute-error]
  In Union[List[nothing], range]
File "/mnt/c/Users/Matt/hg/mercurial/changegroup.py", line 357, in onchangelog:
    No attribute 'update' on None [attribute-error]

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

mharbison72 created this revision.Dec 14 2021, 4:10 PM
Alphare requested changes to this revision.Dec 15 2021, 5:01 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
mercurial/changegroup.py
536

Mhhh now it just looks like nonsensical code. I don't think we should appease Pytype this way

This revision now requires changes to proceed.Dec 15 2021, 5:01 AM
mharbison72 added inline comments.Dec 15 2021, 2:45 PM
mercurial/changegroup.py
536

I didn't like it either, so I'll just make it suppression comments.

FTR, I'm a little concerned about all of the suppression comments (in general, not necessarily here) in case the code changes in a way that it is an issue, but the comments hide it. Also, reusing variables (like foo = str2bytes(foo)) tends to confuse PyCharm, which is a shame because it does a really good job of finding issues. I'm not sure where the sweet spot is.

Alphare accepted this revision.Dec 17 2021, 5:57 AM
Alphare added inline comments.
mercurial/changegroup.py
536

I think these will become easier with the Python 2 removal. We can talk about it more then. :)

This revision is now accepted and ready to land.Dec 17 2021, 5:57 AM
This revision was automatically updated to reflect the committed changes.