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

Mark static code analysis checks deprecated #1613

Merged
merged 2 commits into from
Dec 8, 2018
Merged

Mark static code analysis checks deprecated #1613

merged 2 commits into from
Dec 8, 2018

Conversation

guwirth
Copy link
Collaborator

@guwirth guwirth commented Dec 6, 2018

  • Static code analysis checks will no longer be supported in future versions. Use an external tool for static code analysis instead.

This change is Reviewable

- Static code analysis checks will no longer be supported in future versions. Use an external tool for static code analysis instead.
@guwirth guwirth added this to the 1.2.1 milestone Dec 6, 2018
@guwirth guwirth self-assigned this Dec 6, 2018
@ivangalkin
Copy link
Contributor

@guwirth As far I remember the #1410 was discussed controversially. Does your change make the first step towards the removal?

Copy link
Contributor

@ivangalkin ivangalkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 54 of 54 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guwirth, @jmecosta, and @Bertk)


sonar-c-plugin/src/main/resources/org/sonar/l10n/c/rules/c/BooleanEqualityComparison.html, line 5 at r1 (raw file):

Static code analysis checks will no longer be supported in future versions. Use an external tool for static code analysis instead.

I believe this statement might be misunderstood like "don't use sonar-cxx, replace it with something else". I would write something like

Internal static code analysis checks will no longer be supported in future versions. Sonar-cxx is able to import analysis reports from various external tools (see https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Running-tools). Please consider using external code analyzers instead of deprecated internal ones.

@guwirth
Copy link
Collaborator Author

guwirth commented Dec 6, 2018

As far I remember the #1410 was discussed controversially. Does your change make the first step towards the removal?

@ivangalkin think all agreed there

  • That it makes less sense to add more static code analysis checks.
  • That it is not consistent (and for most user confusing) if we provide a little bit code analysis.

Mark them DEPRECATED seems to be a good compromise to motivate all to switch to external rule checkers.

1 similar comment
@guwirth
Copy link
Collaborator Author

guwirth commented Dec 6, 2018

As far I remember the #1410 was discussed controversially. Does your change make the first step towards the removal?

@ivangalkin think all agreed there

  • That it makes less sense to add more static code analysis checks.
  • That it is not consistent (and for most user confusing) if we provide a little bit code analysis.

Mark them DEPRECATED seems to be a good compromise to motivate all to switch to external rule checkers.

@ivangalkin
Copy link
Contributor

@guwirth ok, I agree. Please consider to re-word the documentation (see my comment above). Besides that I approve the PR.

Copy link
Contributor

@ivangalkin ivangalkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guwirth, @jmecosta, and @Bertk)

@ivangalkin
Copy link
Contributor

@guwirth improve html

thank you

@guwirth guwirth merged commit f403747 into SonarOpenCommunity:master Dec 8, 2018
@jmecosta
Copy link
Member

jmecosta commented Dec 9, 2018

all good from my side.

@guwirth guwirth mentioned this pull request Dec 21, 2018
@guwirth guwirth deleted the deprecated-checks branch December 27, 2018 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants