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

Get rid of default values for the sonar.cxx.<reportsource>.reportPath #331

Closed
wenns opened this issue Dec 22, 2014 · 5 comments · Fixed by #550
Closed

Get rid of default values for the sonar.cxx.<reportsource>.reportPath #331

wenns opened this issue Dec 22, 2014 · 5 comments · Fixed by #550

Comments

@wenns
Copy link
Contributor

wenns commented Dec 22, 2014

I consider those values to be of no real help, introducing them was probably a dumb idea. Also, once we have no default values anymore, we could print warnings into the log if a value is set and yields no files, which is valuable user feedback.

@guwirth
Copy link
Collaborator

guwirth commented Dec 22, 2014

Don't understand the problem?

@wenns
Copy link
Contributor Author

wenns commented Dec 23, 2014

It is a usability improvement. The plugin would be easier to use if there would be a warning in the logs in case one gets the pattern in e.g. "sonar.cxx.cppcheck.reportPath" wrong. Its not the case now, because there is a default value, for cppcheck it is:

private static final String DEFAULT_REPORT_PATH = "cppcheck-reports/cppcheck-result-*.xml";

@guwirth
Copy link
Collaborator

guwirth commented Apr 10, 2015

@wenns does this mean only removing the defaults or do we have to add also checks/warnings?

@wenns
Copy link
Contributor Author

wenns commented Apr 15, 2015

I think this task is composed of following smaller ones:

  • remove the default paths, and all the code around them
  • modify the shouldExecute... to let the sensor run only if the according report property is set
  • add a warning if the property doesnt point to a file
  • adjust the documentation accordingly, mention in the change in the upgrade nodes.

That way, we have (somewhat) smaller logs, a little bit less code, (somewhat) better running times and better usability because of the warnings.

@wenns
Copy link
Contributor Author

wenns commented Apr 15, 2015

By the way: this issue becomes apparent in #278 thread: if the plugin had warned about the report pointing to nothing, analysing the problem for @jfallot would have been a lot easier.

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

Successfully merging a pull request may close this issue.

2 participants