Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools.memories: remove obsolete LGTM tricks #2514

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

dgw
Copy link
Member

@dgw dgw commented Oct 1, 2023

We aren't using LGTM any more, and thus don't need to trick it (46f184e, part of #1754) into suppressing a warning about attributes that were added to SopelMemory vs. the base dict object it's extending.

I don't think this'll do anything in CI, but it fixes a mypy error that seems to only appear on my system. Since there's no technical reason for these assignments to exist, rather than wast time figuring out how to fix the type-check error, let's just remove them.

Like LGTM, the CodeQL static analysis we use now is also difficult/impossible to use effectively on a local machine, so if this brings up any new alerts I/we will have to decide what to do after the fact.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make lint and make test)
    • Fixing an issue reported by make lint locally was in fact the entire reason I made this PR. 😁
  • I have tested the functionality of the things this change touches

Update post-analysis

Yes, now SopelMemory triggers the py/missing-equals rule due to adding a lock attribute without overriding __eq__. IMO removing the existing silly approach is worthwhile without immediately figuring out the best way to resolve the new alert.

We aren't using LGTM any more, and thus don't need to trick it into
suppressing a warning about attributes that were added to `SopelMemory`
vs. the base `dict` object it's extending.

I don't think this'll do anything in CI, but it fixes a mypy error that
seems to only appear on my system. Since there's no technical reason for
these assignments to exist, rather than wast time figuring out how to
fix the type-check error, let's just remove them.
@dgw dgw added the Housekeeping Code cleanup, removal of deprecated stuff, etc. label Oct 1, 2023
@dgw dgw added this to the 8.0.0 milestone Oct 1, 2023
@dgw dgw requested a review from a team October 1, 2023 23:25
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah!

@dgw dgw merged commit f2e1366 into master Oct 7, 2023
7 checks passed
@dgw dgw deleted the memories-obsolete-lgtm-tricks branch October 7, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping Code cleanup, removal of deprecated stuff, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants