Addressing code review comments for debian identification (#115)

[rbuccigrossi]

No description provided.

[codecov-io]

Codecov Report

Merging #131 into master will increase coverage by 0.59%.
The diff coverage is 92.3%.

@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
+ Coverage   76.41%   77.01%   +0.59%     
==========================================
  Files         119      117       -2     
  Lines        8363     8208     -155     
==========================================
- Hits         6391     6321      -70     
+ Misses       1972     1887      -85

Impacted Files	Coverage Δ
niceman/support/tests/test_external_versions.py	86.44% <ø> (ø)	⬆️
niceman/support/distributions/tests/test_debian.py	100% <ø> (ø)	⬆️
niceman/support/distributions/debian.py	95% <100%> (+0.1%)	⬆️
niceman/distributions/tests/test_debian.py	98.09% <100%> (+0.01%)	⬆️
niceman/interface/retrace.py	98.33% <100%> (ø)	⬆️
niceman/support/external_versions.py	92.77% <100%> (+0.17%)	⬆️
niceman/resource/tests/test_shell.py	100% <100%> (ø)	⬆️
niceman/distributions/debian.py	89.27% <100%> (+0.59%)	⬆️
niceman/tests/utils.py	79.64% <83.33%> (+0.46%)	⬆️
niceman/distributions/vcs.py	80% <0%> (-4.09%)	⬇️
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 11abdd2...496efe8. Read the comment docs.

[yarikoptic]

insignificant "styling" comment: python allow trailing , between items of the list, dict, or tuple. So I usually add it to minimize the diff next time (since I wouldn't need to add it to preceding line)

[yarikoptic]

I believe I added it on the commit but it doesn't show here -- could we have a test which tests that value is parsed correctly?

[rbuccigrossi]

That is a quote right from your bug report, but I cut out the exact portion that caused the problem. I'm happy to add more if you wish...

[yarikoptic]

What I meant -- could the test verify correct parsing of this line -- that we are getting correct record with origin="Debian Backports" etc?

PS as for what "doesn't show here" -- it was about my comment, not the code ;)

[rbuccigrossi]

Done ( 496efe8 )

[yarikoptic]

I think we could merge as soon as wip label disappears ;-)

[rbuccigrossi]

@yarikoptic I removed the WIP tag. There's only one thing left in your original review, and that's the "sledgehammer test". I'll do that as a separate pull request. So if you're happy, merge away!