Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Cleaned up typing for linter errors in taskkeeper #4122

Merged
merged 2 commits into from
Apr 19, 2019

Conversation

maaktweluit
Copy link
Contributor

No description provided.

@jiivan
Copy link
Contributor

jiivan commented Apr 18, 2019

IMHO it looks more like namespace pollution than clean up to me. :)

@maaktweluit
Copy link
Contributor Author

@jiivan that was based on this comment: https://github.com/golemfactory/golem/pull/4120#discussion_r275848258

Will change it according to the solution there then too :)

@maaktweluit maaktweluit force-pushed the mwu/taskkeeper-mypy branch from bf895e1 to 1beb62a Compare April 18, 2019 12:38
@maaktweluit
Copy link
Contributor Author

@jiivan updated the PR

IMHO readable code is more important then a clean namespace

Copy link
Contributor

@mfranciszkiewicz mfranciszkiewicz left a comment

Choose a reason for hiding this comment

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

LGTM.

Improvement suggestion - maybe specify types for the nested collections?

@codecov
Copy link

codecov bot commented Apr 18, 2019

Codecov Report

Merging #4122 into develop will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #4122      +/-   ##
===========================================
+ Coverage    88.67%   88.68%   +<.01%     
===========================================
  Files          214      214              
  Lines        18697    18697              
===========================================
+ Hits         16580    16581       +1     
+ Misses        2117     2116       -1

Copy link
Contributor

@jiivan jiivan left a comment

Choose a reason for hiding this comment

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

Readibility is subjective :)

@maaktweluit
Copy link
Contributor Author

@mfranciszkiewicz good one! did not see the typeception, will add those 2

@maaktweluit maaktweluit force-pushed the mwu/taskkeeper-mypy branch from 1beb62a to 79dcd66 Compare April 19, 2019 08:10
@maaktweluit maaktweluit merged commit 8fd86f0 into develop Apr 19, 2019
@ghost ghost removed the in progress label Apr 19, 2019
@maaktweluit maaktweluit deleted the mwu/taskkeeper-mypy branch April 19, 2019 09:01
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants