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

remove with_mypy or with-mypy extension/optional tool for prospector #2108

Merged
merged 3 commits into from Jun 22, 2023

Conversation

valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Jun 21, 2023

Description

There is an issue with prospector=1.10.2 at the moment and installing it with with_mypy makes it not work at all, so I am removing this extra tool for now, so we have some functionality out of it, rather than a complete shutdown;

Nevermind, prospectpr=1.10.2 needs with-mypy instead of with_mypy due to poetry changing version, and @carlio (the prospector main dev) tells me this may change in the future again, depending how poetry wakes up in the morning (what version it is one)

@bouweandela suggested we remove the extra tool option altogether since we run the mypy tests separately anyway 👍

See landscapeio/prospector#624


Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number pull requests:

@valeriupredoi valeriupredoi added the installation Installation problem label Jun 21, 2023
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #2108 (99d4aa8) into main (cf3824e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2108   +/-   ##
=======================================
  Coverage   93.08%   93.08%           
=======================================
  Files         237      237           
  Lines       12793    12793           
=======================================
  Hits        11909    11909           
  Misses        884      884           

@bouweandela
Copy link
Member

We can just drop the with_mypy/with-mypy extra because we're installing mypy separately already.

@valeriupredoi
Copy link
Contributor Author

aha! Let me do that now - even better! 🍺

@valeriupredoi valeriupredoi changed the title Temporraily remove with_mypy extension/optional tool for prospector remove with_mypy or with-mypy extension/optional tool for prospector Jun 22, 2023
@bouweandela
Copy link
Member

@bouweandela suggested we remove the extra tool option altogether since we run the mypy tests separately anyway +1

What I actually meant is that we're installing mypy as a separate tool as well, so there is no need to pull it in through the prospector with_mypy extra. Because it is installed, it works fine with prospector too.

@valeriupredoi
Copy link
Contributor Author

@bouweandela suggested we remove the extra tool option altogether since we run the mypy tests separately anyway +1

What I actually meant is that we're installing mypy as a separate tool as well, so there is no need to pull it in through the prospector with_mypy extra. Because it is installed, it works fine with prospector too.

indeed - mypy is a core dep for us here - may want to deouble check in Tool though

@valeriupredoi
Copy link
Contributor Author

thanks bud 🍺

@valeriupredoi valeriupredoi merged commit 26b50fa into main Jun 22, 2023
4 checks passed
@valeriupredoi valeriupredoi deleted the remove_with_mypy_extension_prospector branch June 22, 2023 12:43
@bouweandela bouweandela added this to the v2.9.0 milestone Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
installation Installation problem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants