-
Notifications
You must be signed in to change notification settings - Fork 18
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
Specify baseline level for checks #31
Comments
Oh, and please take for granted that if something isn't marked as assigned, PRs are definitely welcome (and probably even when they're marked as assigned, frankly) :) |
I was just coming here to suggest this. :) (Although I think 100 lines max is a better default) Secondly I think this should be changed to only return +1 for each file above the limit, instead of the sum of all lines in all files. I was very confused when quality told me there was 149 bigfiles violations, however bigfiles was only outputting three files and I knew I didn't even have 149 files in the directory. I had no idea where that number was coming from till I looked at the code above.
|
Gotcha! I would recommend for that type of check ("I want to ratchet down the number of files over a certain size"), you use RuboCop with the quality gem. RuboCop has a maximum-class-size check which you could configure as you'd like to, and the quality gem would make sure the total number of those decrease with time. The bigfiles check in the quality gem is trying to do something different--find your largest, most complex files and provide a cue for you to refactor them when you actually touch them (and increase their size as a result). The purpose of this issue was to basically provide a cut-off of 'my files, even the three biggest, are small enough; your job here is done'. Documentation clarifying the purpose of each tool used by quality would certainly be welcome (and the bigfiles repo at https://www.github.com/apiology/bigfiles could probably use a bit of help as well). Thoughts? Curious whether I understand your use case. |
I might be missing your point but... Ok say the three biggest files in a project are each 20 lines long. You will be told "Found 60 bigfiles violations". This says to the developer, there are 60 things wrong with their codebase. When I'd say most people would think there are actually zero file size or complexity problems in this scenario. Even if somebody decided that 20 lines is too big for a file, they still wouldn't consider this to be 60 violations, but instead 3 violations. To me the problem isn't with bigfiles, but how it's results are used in quality. Half of this seemed to be what you were saying in your first comment on this issue. |
I think what you're saying is that the wording could be better in quality for checks like bigfiles--in some checks, violations is the thing they output. With some checks, it's a score which the quality gem measures against a threshold, and the term 'violations' doesn't really apply. Does that capture it? |
Well I guess "bigfiles: found a total of 60 lines in your three biggest files" would be better as this can be understood without going into the quality source code. |
Thanks for the input! I'd definitely be open to a PR expressing a well thought out and general way of expressing the violations vs threshold nature of different types of checks. |
There are some checks, like BigFiles, where I don't really care so long as things are below a certain level--I'm not trying to drive them down to zero. For BigFiles, for instance, so long as my top three files added together are, say, 300 lines or fewer, I'm happy. I'd like to teach this to the tool so that it doesn't ratchet down below a given letter.
The text was updated successfully, but these errors were encountered: