raimon49 / pip-licenses

Dump the license list of packages installed with pip.
MIT License
307 stars 43 forks source link

Add type annotations #137

Closed b-kamphorst closed 1 year ago

b-kamphorst commented 1 year ago

Hi @raimon49, this PR looks intimidating so please have a look at the commits one-by-one. For example, just running a linter already caused a huge diff. I'm not entirely happy with the type hints but this is as far as I could go without actually changing the logic.

I think type annotations would have helped me a lot for my first contribution, and it will also make it easier to make future contributions. Of course that is subjective. I can imagine that you're not quite looking for a PR like this, if so I'll just close the PR. Looking forward to your thoughts.

codecov[bot] commented 1 year ago

Codecov Report

Merging #137 (e9413fd) into master (6c49185) will decrease coverage by 0.26%. The diff coverage is 99.13%.

@@             Coverage Diff             @@
##            master     #137      +/-   ##
===========================================
- Coverage   100.00%   99.73%   -0.27%     
===========================================
  Files            1        1              
  Lines          371      377       +6     
===========================================
+ Hits           371      376       +5     
- Misses           0        1       +1     
Impacted Files Coverage Δ
piplicenses.py 99.73% <99.13%> (-0.27%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

raimon49 commented 1 year ago

@b-kamphorst Thanks, this diff looks good to me, I am glad that you have shown the utmost respect for this Git repo style.

It's a good idea to introduce a code formatter. This PR would be acceptable.