Skip to content

Conversation

@koesie10
Copy link
Member

@koesie10 koesie10 commented Mar 1, 2023

This is definitely not a perfect solution since we're essentially just moving the place where we're casting. However, because we have manually made the types similar, this provides some type assurances where there were none before. This also has the cast in only one place, which makes it easier to find and fix in the future.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

This is definitely not a perfect solution since we're essentially just
moving the place where we're casting. However, because we have manually
made the types similar, this provides some type assurances where there
were none before. This also has the cast in only one place, which makes
it easier to find and fix in the future.
@koesie10 koesie10 added the secexp label Mar 1, 2023
@koesie10 koesie10 marked this pull request as ready for review March 1, 2023 11:34
@koesie10 koesie10 requested review from a team as code owners March 1, 2023 11:34
Copy link
Contributor

@elenatanasoiu elenatanasoiu left a comment

Choose a reason for hiding this comment

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

Looks like mockedQuickPickItem extends QuickPickItem directly rather than forcing the cast through as unknown as so it's definitely better in terms of type-checking (as far as I can tell).

So I think that means we're addressing our linting issue with the same pattern of creating a mock object, even if it has some different mocking requirements.

@koesie10 koesie10 merged commit 9703d10 into main Mar 3, 2023
@koesie10 koesie10 deleted the koesie10/remove-as-unknown-as-quickpickitem branch March 3, 2023 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants