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

Report on files being validated against #346

Open
rnveach opened this issue Nov 10, 2022 · 7 comments
Open

Report on files being validated against #346

rnveach opened this issue Nov 10, 2022 · 7 comments

Comments

@rnveach
Copy link
Member

rnveach commented Nov 10, 2022

Identified at checkstyle/eclipse-cs#380 (comment) ,

It was not clear why we were getting random issues and then it became clear patch-filters prevented a Checkstyle run in some cases, which is its purpose.

It would be helpful if patch-filters reported something minor like "No files identified as changed" or "Identified X files as changed" so it is clear when allows Checkstyle to run and not run.

I don't think it would have helped this case, but it will make what it is doing clearer.

@romani
Copy link
Member

romani commented Nov 10, 2022

It just another filter for checkstyle.
We do not have ability communicate to execution module. Can you share more details on how you see it !

@rnveach
Copy link
Member Author

rnveach commented Nov 10, 2022

Currently output is:

[INFO] --- maven-checkstyle-plugin:3.2.0:check (checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] You have 0 Checkstyle violations.
[INFO] 
[INFO] --- maven-checkstyle-plugin:3.2.0:check (sevntu-checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] You have 0 Checkstyle violations.
[INFO] 
[INFO] --- maven-install-plugin:2.3.1:install (default-install) @ net.sf.eclipsecs-updatesite ---

Following is what I think should be displayed:

[INFO] --- maven-checkstyle-plugin:3.2.0:check (checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] Patch-Filters prevented any files from being validated.
[INFO] You have 0 Checkstyle violations.
[INFO] 
[INFO] --- maven-checkstyle-plugin:3.2.0:check (sevntu-checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] Patch-Filters allowed 1 file(s) to be validated.
[INFO] You have 0 Checkstyle violations.
[INFO] 
[INFO] --- maven-install-plugin:2.3.1:install (default-install) @ net.sf.eclipsecs-updatesite ---

But your right, as just a normal filter this is not currently possible. This would either have to be more of a BeforeExecutionFileFilter, which would still require it to be allowed to print output like an Audit Listener, or create a separate Audit Listener to display notifications all together. A second module is not how I would want this to go, but the only other option then is a change to allow a filter to also be an audit listener (implement Filter and AuditListener, both are interfaces).

Also displaying number of files seems pointless now, as patch-filters slims down the file to where the changes are, so its not the full file validation. I guess my final recommendation then, would be the number of violations patch-filters prevented.

[INFO] --- maven-checkstyle-plugin:3.2.0:check (checkstyle-check) @ net.sf.eclipsecs-updatesite ---
[INFO] Patch-Filters suppressed XXX violation(s) in unchanged files and lines.
[INFO] You have 0 Checkstyle violations.
[INFO] 

@romani
Copy link
Member

romani commented Nov 11, 2022

Output that you reference is output of plugin, not a core checkstyle library

@rnveach
Copy link
Member Author

rnveach commented Nov 11, 2022

Trimmed from my examples above, the output I am asking from this plugin is one of the following:

Patch-Filters prevented any files from being validated.
Patch-Filters allowed 1 file(s) to be validated.
Patch-Filters suppressed XXX violation(s) in unchanged files and lines.

I am just looking for some feedback form patch-filters on what it is doing so its a bit clearer to the user(s) who want to know.

@romani
Copy link
Member

romani commented Nov 11, 2022

asking from this plugin

How you envision filter to print output to plugin output?
This project is just filter implementation.

@rnveach
Copy link
Member Author

rnveach commented Feb 4, 2023

Another reason to provide some feedback is to show progress to removing all violations for reporting. The purpose of the filter is to allow users to resolve new violations slowly.

There is no way to know right now how many violations are left, or when all violations are now resolved and project team can discuss removing suppression filter if they wish. It may be beneficial for this number to be seen to show management that they are now fully in compliance with all styling for project.

Also, upgrading to new version of Checkstyle or adding new modules will not show how many new violations are being added also, as the only thing initially changing in commit is the configuration file.

@rnveach
Copy link
Member Author

rnveach commented Feb 4, 2023

How you envision filter to print output to plugin output?

This will most likely require a lifecycle change from main project for proper implementation. I am open to ideas.

Maybe all modules can be given a custom output function where the data makes it way to AuditListener to handle and print.

This may just be similar to a violation log (with info severity), so maybe all type of modules, including filters, should be allowed to print violations. The only difference is this type of printing discussed here won't have a file attached to it.

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

No branches or pull requests

2 participants