Skip to content

Simplify non-verbose rule output #207

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

Merged
merged 3 commits into from
Jul 19, 2021
Merged

Simplify non-verbose rule output #207

merged 3 commits into from
Jul 19, 2021

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Jul 19, 2021

Previously, the output resulting from rule violations was always prefaced with the rule ID and result:

Rule LD002 result: fail

This is necessary in verbose output mode because in that mode the result of every rule applied to the project is printed.
This is the only way to identify the rule and result when a rule passes.

The situation is different non-verbose mode, where output is only printed when a rule is violated. This means that:

  • The rule is identified by the rule message
  • The "fail" rule result is implicit

So this preface doesn't really serve any purpose in non-verbose mode. It is somewhat cryptic, so may make the output less approachable to the users. Because the rule ID is still sometimes of value as a succinct and unequivocal way to refer to a specific rule, it is printed in a less prominent location at the end of the rule message.


Example of non-verbose rule output before this change:

Rule LD002 result: fail
WARNING: No license file found. See: https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/licensing-a-repository#detecting-a-license

after:

WARNING: No license file found. See: https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/licensing-a-repository#detecting-a-license (Rule SD002)

per1234 added 2 commits July 19, 2021 02:46
An error in the assertion arguments resulted in it providing meaningless results. This also masked the fact that its
intended assertion was outdated.
Since output always has had the same format regardless of the tool configuration, with the configuration only determining
whether or not it will be printed, it made sense to place the printing control logic at the point of printing, always
outputting the message from the result package.

It has now been determined that there are different formatting requirements for verbose and non-verbose output. This
means that it makes more sense to move that logic to the result package and simply return an empty string when no output
should be printed. This refactoring in preparation for the formatting changes to the non-verbose output.
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jul 19, 2021
@per1234 per1234 requested review from silvanocerza and umbynos July 19, 2021 10:47
@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2021

Codecov Report

Merging #207 (64551f8) into main (8277f9e) will increase coverage by 0.03%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #207      +/-   ##
==========================================
+ Coverage   87.86%   87.90%   +0.03%     
==========================================
  Files          43       43              
  Lines        4146     4150       +4     
==========================================
+ Hits         3643     3648       +5     
+ Misses        392      391       -1     
  Partials      111      111              
Flag Coverage Δ
unit 87.90% <90.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/rule/rule.go 52.00% <0.00%> (+2.00%) ⬆️
internal/result/result.go 88.49% <100.00%> (+0.64%) ⬆️
internal/rule/rulefunction/library.go 94.75% <0.00%> (-0.01%) ⬇️

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 8277f9e...64551f8. Read the comment docs.

Previously, the output resulting from rule violations was always prefaced with the rule ID and result:

Rule SD001 result: fail

This is necessary in verbose output mode because in that mode the result of every rule applied to the project is printed.
This is the only way to identify the rule and result when a rule passes.

The situation is different non-verbose mode, where output is only printed when a rule is violated. This means that:

- The rule is identified by the rule message
- The rule result is implicit

So this preface doesn't really serve any purpose in non-verbose mode. It is somewhat cryptic, so may make the output less
approachable to the users. Because the rule ID is still sometimes of value as a succinct and unequivocal way to refer to
a specific rule, it is printed in a less prominent location at the end of the rule message.
@per1234 per1234 merged commit dccb221 into arduino:main Jul 19, 2021
@per1234 per1234 deleted the simplify-non-verbose-output branch July 19, 2021 11:12
@per1234 per1234 self-assigned this Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants