urlchecker-python icon indicating copy to clipboard operation
urlchecker-python copied to clipboard

Print level feature

Open SuperKogito opened this issue 5 years ago • 4 comments

  • This PR solves #29
  • This PR adds a --print-level feature and removes the old --print-all flag.
  • The --print-level has 4 printing levels:
    • all: all prints
    • files-with-urls-only: print only for files with urls in them.
    • fails-only: print only broken links.
    • success-only: print only working links.
    • none: no files or urls prints.
  • The tests and README are adjusted accordingly.

SuperKogito avatar Apr 13 '20 14:04 SuperKogito

Codecov Report

Merging #33 into master will increase coverage by 0.67%. The diff coverage is 89.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
+ Coverage   77.03%   77.70%   +0.67%     
==========================================
  Files          20       20              
  Lines         627      628       +1     
==========================================
+ Hits          483      488       +5     
+ Misses        144      140       -4     
Impacted Files Coverage Δ
urlchecker/client/check.py 31.14% <0.00%> (+6.55%) :arrow_up:
urlchecker/core/urlproc.py 92.92% <88.23%> (+0.45%) :arrow_up:
tests/test_client_check.py 97.56% <100.00%> (-0.12%) :arrow_down:
tests/test_core_check.py 100.00% <100.00%> (ø)
tests/test_core_urlproc.py 100.00% <100.00%> (ø)
urlchecker/client/__init__.py 75.92% <100.00%> (ø)
urlchecker/core/check.py 83.01% <100.00%> (ø)

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 2279da0...f14fc12. Read the comment docs.

codecov[bot] avatar Apr 13 '20 14:04 codecov[bot]

Awesome! I’ll make some time to review this week.

vsoch avatar Apr 13 '20 15:04 vsoch

I think I’ve changed my mind about this feature - given the core use of url checker for CI runs, it feels like feature bloat, and I’m not convinced that this is the cleanest implementation to insert a bunch of random if/else all over the library. Implementing this with a logger type interface that calls a statement based on a level would be much more elegant. I think the work is still valuable and we should keep the PR for future feedback but I am not wanting to add this now to the library. Thanks for your work on this @SuperKogito though!

vsoch avatar Apr 14 '20 13:04 vsoch

okay xD what made this sudden change of heart?

SuperKogito avatar Apr 14 '20 13:04 SuperKogito