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

Review code quality rule configuration and suppressions (after R24.05) #547

Open
dsmf opened this issue Apr 17, 2024 · 0 comments
Open

Review code quality rule configuration and suppressions (after R24.05) #547

dsmf opened this issue Apr 17, 2024 · 0 comments

Comments

@dsmf
Copy link
Contributor

dsmf commented Apr 17, 2024

As developer,
I want to review (and if necessary adjust) code quality rules and the existing suppressions,
so that the rules really improve code quality and do not lead to a lot of SuppressWarnings (which is contraproductive) and technical debt is cleaned up.

Hints / Details

Current SuppressWarnings:

     18 PMD.ExcessiveImports         <-- limit too low?
     16 PMD.TooManyMethods           <-- IMHO limit too low, leads to SuppressWarnings or duplicating code instead of extraction to methods

     10 PMD.ShortVariable                 <-- rule is ok and but some SuppressWarnings should always be on attribute level, not on class level
      8 PMD.UseObjectForClearerAPI
      7 PMD.AvoidCatchingGenericException <-- code places should be reviewed whether the suppression is really ok

      5 PMD.AvoidDuplicateLiterals        <-- all suppressions seem ok, only in controllers, probably because of OpenAPI
      5 PMD.ShortClassName              <-- most suppressions intended and ok, at least one superfluous because class name is not too short (JobErrorDetails)

      4 PMD.PreserveStackTrace          <-- review code places

      3 PMD.UseConcurrentHashMap

      3 PMD.ShortMethodName             <-- should be over each method that has a short name intentionally, not over class, code places seem to be except AspectType, where the suppression does not seem to be necessary at all

      2 PMD.XXX
      2 PMD.TooManyStaticImports 
      1 PMD.UseUtilityClass

      1 PMD.UnusedFormalParameter <-- suppression on PaginatedResponse no longer necessary?
      1 PMD.SignatureDeclareThrowsException <-- ok

      1 PMD.DoNotUseThreads

      2 PMD.UseVarargs
      1 PMD.FieldNamingConventions          <-- superfluous suppression in RelationshipAspect
      1 PMD.DataClass
      1 PMD.CyclomaticComplexity               <-- ok

      1 PMD.AvoidRethrowingException        <-- review code place, if ok, add comment with reason

      1 PMD.MethodArgumentCouldBeFinal    <-- superfluous on JobErrorDetails

besides the PMD rules there is also a checkstyle rule concerning too long comment that does not match with our code line length (which is annoying and slows down dev because the comment must be shorter that the code margin)

some suppressions seem unnecessary:

  • suppressions on JobErrorDetails can be removed completely
  • ...

Acceptance Criteria

  • PMD.ExcessiveImports and PMD.TooManyMethods configured in a way that leads to better code quality and not to a lot of suppressions or code duplications just to avoid the rule
  • Suppressions are reviewed whether ok, if not cleaned up or PBI for cleanup is created
  • SuppressWarnings on class level are avoided wherever possible (move to attribute / method level if possible)
  • Comment too long is harmonized with code margin

Out of Scope

  • ...
@dsmf dsmf added this to IRS Apr 17, 2024
@dsmf dsmf moved this to backlog in IRS Apr 17, 2024
@dsmf dsmf moved this from backlog to inbox in IRS Apr 17, 2024
@dsmf dsmf changed the title Review code quality rule configuration and suppressions Review code quality rule configuration and suppressions (after R24.05) Apr 17, 2024
ds-jhartmann pushed a commit to ds-jhartmann/item-relationship-service that referenced this issue Jun 13, 2024
…-helm-release-6.6.1

Prepare Helm release for next version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: inbox
Development

No branches or pull requests

1 participant