Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "--short" option, print in warnings in red or green #359

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

bzgec
Copy link

@bzgec bzgec commented Sep 4, 2022

  1. Add "--short" option prints only warnings and total summary.

  2. Print warnings string:

  • red (threshold exceeded) - !!!! Warnings (cyclomatic_complexity > 15 or length > 1000 or nloc > 1000000 or parameter_count > 100) !!!!
    (problematic functions are not printed in red)
  • green (no threshold exceeded) - No thresholds exceeded (cyclomatic_complexity > 15 or length > 1000 or nloc > 1000000 or parameter_count > 100)

Missing "--short" option tests

@terryyin
Copy link
Owner

Hi @bzgec
thanks for the pull request and sorry for the late response.
Do you have test for "--short" option now?

@bzgec
Copy link
Author

bzgec commented May 21, 2023

Are tests from 02da78e enough?

@terryyin
Copy link
Owner

Hi @bzgec thanks for the test. I have some comments on the test and solution. Some of them are preventive unless my assumptions are wrong.

  1. I think it's an improvement to split print_and_save_modules into save_modules and save_modules. But save_modules seems to be a method that is blocking, rather than continuous yielding. The behaviour of lizard will be very different after this change.
  2. Since the change is mainly adding one if, having one new test is ok. However, assertNotEqual is something I will try my best to avoid. It's too easy to pass and too hard to fail, especially when you are asserting a very specific thing. I find the project so far has 0 assertNotEqual.

@bzgec
Copy link
Author

bzgec commented May 22, 2023

Ok, so for number 2 this should be better? - 51151e4

Did I understand correctly what you had in mind for the number 1? - ed131da

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants