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

deprecate *PUBLIC*API* metrics #1510

Closed

Conversation

ivangalkin
Copy link
Contributor

@ivangalkin ivangalkin commented Jun 28, 2018

  • PUBLIC_API, PUBLIC_UNDOCUMENTED_API and PUBLIC_DOCUMENTED_API_DENSITY
    were depricated since SQ 6.2 (https://jira.sonarsource.com/browse/SONAR-8328)
    but existed as custom sonar-cxx metrics

  • the minimal supported SQ version now is 6.7

  • moreover, it looks like there is a general problem in displaying
    custom metrics (see sonar-cxx custom metrics are not dispayed/saved #1509)

  • that means that nobody a) expects that the SQ plugin implements the
    deprectad metrics and b) nobody misses them (because they are just not
    visualized)

  • BTW public API measurements belong to the obligatory AST Visitors
    and introduce the time overhead of ~6% (measured with introduce CxxAstVisitorProfiler for CxxAstScanner #1507,
    6% means, that if there is no sensors activated at all
    the importing of C++ project will still cause a calculation
    of a bunch of metrics; summary overhead of this calculation
    considered as 100%)


This change is Reviewable

@ivangalkin ivangalkin requested review from guwirth and Bertk June 28, 2018 10:17
@ivangalkin ivangalkin force-pushed the deprecate_public_api branch from 923cd00 to ad0da49 Compare June 28, 2018 10:32
* `PUBLIC_API`, `PUBLIC_UNDOCUMENTED_API` and `PUBLIC_DOCUMENTED_API_DENSITY`
  were depricated since SQ 6.2 (https://jira.sonarsource.com/browse/SONAR-8328)
  but existed as custom sonar-cxx metrics

* the minimal supported SQ version now is 6.7

* moreover, it looks like there is a general problem in displaying
  custom metrics (see SonarOpenCommunity#1509)

* that means that nobody a) expects that the SQ plugin implements the
  deprectad metrics and b) nobody misses them (because they are just not
  visualized)

* the squid check `UndocumentedApiCheck` is not affected
  (it doesn't rely on the stored metric, but visits the AST
  by itself) but `CxxPublicApiVisitorTest` had to be rewritten

* BTW public API measurements belong to the obligatory AST Visitors
  and introduce the time overhead of ~6% (measured with SonarOpenCommunity#1507,
  6% means, that if there is no sensors activated at all
  the importing of C++ project will still cause a calculation
  of a bunch of metrics; summary overhead of this calculation
  considered as 100%)
@ivangalkin ivangalkin force-pushed the deprecate_public_api branch from ad0da49 to f176ec0 Compare June 28, 2018 12:15
@ivangalkin
Copy link
Contributor Author

AbstractCxxPublicApiVisitor and the corresponding tests CxxPublicApiVisitorTest (must be renamed) can be moved from cxx-squid to cxx-checks too. If there will be general agreement w.r.t. this PR, I will update the change. Please review.

@guwirth
Copy link
Collaborator

guwirth commented Jun 29, 2018

Hi @ivangalkin, there was by intention a custom metric introduced. Think there is much value to detect undocumented public API. This is for sure technical debt. Never understood why SonarSource removed it? What’s correct: if someone don’t use it he should not pay for it. Maybe we can check if rule i s active. Regards,

@guwirth
Copy link
Collaborator

guwirth commented Jun 29, 2018

Alternativ would be #1062

@ivangalkin
Copy link
Contributor Author

will be abandoned as soon as #1509 is finished

@ivangalkin
Copy link
Contributor Author

abandon in favor of #1514

@ivangalkin ivangalkin closed this Jul 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants