crocs-muni / sec-certs

Tool for analysis of security certificates and their security targets (Common Criteria, NIST FIPS140-2...).
https://sec-certs.org
MIT License
9 stars 7 forks source link

Replace linters with ruff #308

Closed adamjanovsky closed 1 year ago

codecov[bot] commented 1 year ago

Codecov Report

Base: 73.31% // Head: 72.81% // Decreases project coverage by -0.49% :warning:

Coverage data is based on head (468b63f) compared to base (0d4e52c). Patch coverage: 86.24% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #308 +/- ## ========================================== - Coverage 73.31% 72.81% -0.49% ========================================== Files 45 45 Lines 5592 5545 -47 ========================================== - Hits 4099 4037 -62 - Misses 1493 1508 +15 ``` | [Impacted Files](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni) | Coverage Δ | | |---|---|---| | [src/sec\_certs/cli.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy9jbGkucHk=) | `0.00% <ø> (ø)` | | | [src/sec\_certs/model/evaluation.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy9tb2RlbC9ldmFsdWF0aW9uLnB5) | `0.00% <0.00%> (ø)` | | | [src/sec\_certs/sample/fips\_iut.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy9zYW1wbGUvZmlwc19pdXQucHk=) | `89.78% <ø> (ø)` | | | [src/sec\_certs/utils/pandas.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy91dGlscy9wYW5kYXMucHk=) | `0.00% <0.00%> (ø)` | | | [src/sec\_certs/utils/parallel\_processing.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy91dGlscy9wYXJhbGxlbF9wcm9jZXNzaW5nLnB5) | `100.00% <ø> (ø)` | | | [src/sec\_certs/model/cpe\_matching.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy9tb2RlbC9jcGVfbWF0Y2hpbmcucHk=) | `79.24% <57.15%> (-0.43%)` | :arrow_down: | | [src/sec\_certs/dataset/cpe.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy9kYXRhc2V0L2NwZS5weQ==) | `54.39% <75.00%> (-0.78%)` | :arrow_down: | | [src/sec\_certs/dataset/fips\_algorithm.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy9kYXRhc2V0L2ZpcHNfYWxnb3JpdGhtLnB5) | `66.28% <75.00%> (-20.08%)` | :arrow_down: | | [src/sec\_certs/utils/helpers.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy91dGlscy9oZWxwZXJzLnB5) | `80.16% <75.00%> (-0.29%)` | :arrow_down: | | [src/sec\_certs/sample/fips.py](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni#diff-c3JjL3NlY19jZXJ0cy9zYW1wbGUvZmlwcy5weQ==) | `86.83% <78.58%> (-0.48%)` | :arrow_down: | | ... and [22 more](https://codecov.io/gh/crocs-muni/sec-certs/pull/308?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=crocs-muni)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

adamjanovsky commented 1 year ago

@J08nY could you pls take a look at Section 3 at https://github.com/charliermarsh/ruff#table-of-contents ? I'd consider introducing more checks, but some are very opinionated. I've went through the list and these are some that I propose:

Thoughts? These can be easily added via ruff.

J08nY commented 1 year ago

I wouldn't go for this. We don't have annotations in places where they are too hard or too messy and I don't want to be forced to handle that.

I mean I like pathlib but similar to the above, sometimes it does not make sense. I am not sure what this will do with our code. If you see the results and think they are sensible/fixable then maybe?

Yeah why not, but make it not fail the build just tell me.

I mean whatever.

I don't care for lambdas much, sometimes some things are more readable than others. Similarly than above, lets see what the results are on the code and whether we like the proposed changes.

adamjanovsky commented 1 year ago

@J08nY

https://pypi.org/project/flake8-annotations/

this is really premature, never meant to implement it, don't know why I listed it 😆

https://pypi.org/project/flake8-use-pathlib/

Resulted in just few suggestions, gonna enforce it .

https://pypi.org/project/flake8-return/

Many opinionated suggestions, implemented some but will not introduce this as a check.

https://github.com/charliermarsh/ruff#flake8-comprehensions-c4

simplified few things, I'll keep it there.

https://pypi.org/project/flake8-simplify/

quite nice changes, not opinionated. I don't thing I can set this not to fail. I started enforcing it. If we ever find some problematic instance or we feel it's more trouble than benefit, we can disable.

All together its -100 LoC while improving readability.