Skip to content

Conversation

@sdavtaker
Copy link

Description

Describe:

  • This pull request add type-hints for mypy to be able to validate the codebase has not typing issues.

The code was tested by running mypy before and after, there is no behavioural change.

Before:

giturlparse/platforms/base.py:22: error: Need type annotation for "DEFAULTS" (hint: "DEFAULTS: dict[<type>, <type>] = ...")  [var-annotated]
giturlparse/platforms/gitlab.py:31: error: Incompatible types in assignment (expression has type "tuple[str, str]", base class "BasePlatform" defined the type as "None")  [assignment]
giturlparse/platforms/github.py:27: error: Incompatible types in assignment (expression has type "tuple[str, str]", base class "BasePlatform" defined the type as "None")  [assignment]
giturlparse/platforms/friendcode.py:5: error: Incompatible types in assignment (expression has type "tuple[str]", base class "BasePlatform" defined the type as "None")  [assignment]
giturlparse/platforms/bitbucket.py:19: error: Incompatible types in assignment (expression has type "tuple[str]", base class "BasePlatform" defined the type as "None")  [assignment]
giturlparse/platforms/assembla.py:5: error: Incompatible types in assignment (expression has type "tuple[str]", base class "BasePlatform" defined the type as "None")  [assignment]
Found 6 errors in 6 files (checked 15 source files)

After

mypy .                                 
Success: no issues found in 15 source files

To be able to run the checks, mypy and types sub for invoke and setuptools are required.

pip install mypy
pip install types-invoke
pip install types-setuptools

Checklist

  • Code lint checked via inv lint
  • [-] Tests added

No test added since its not adding functionallity.
I was not able to run inv lint, neither precommit. I run into some issues trying to run it, I will open a Issue about it.

I also tried to follow the format specified here for the commit and changes in the changes file but couldn't understand where should I get the from.
If you can guide me a little I can update the PR to adhere to the convention.

Copy link

@ivs-cetmix ivs-cetmix left a comment

Choose a reason for hiding this comment

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

LGTM

@ivs-cetmix
Copy link

Hi @protoroto same question here - any changes to have it merged?

@protoroto protoroto mentioned this pull request Oct 22, 2025
@protoroto
Copy link
Member

@ivs-cetmix I'll try to contact the pr author to make a few changes in order to make the CI pass correctly.

@sdavtaker Thanks for contributing! In order to make the CI pass, I have opened an issue and you should:

  • create a branch named feature/issue-121-add-type-hints (or whatever you feel like, but the feature/issue-121- part is the important part)
  • create a file inside changes directory , named 121.feature , with a brief description of the changes made (something along with Adding type hints to be mypy compliant is totally fine)

@ivs-cetmix
Copy link

@protoroto thank you very much!

@sdavtaker
Copy link
Author

Hi,
Created #122 with the new branch, GH doesn't allow to change base branch in the PR.
Added the missing file.

@sdavtaker sdavtaker closed this Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants