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

[TECHNICAL] Detekt: static code analyzer #4487

Merged
merged 74 commits into from
Jan 31, 2025
Merged

[TECHNICAL] Detekt: static code analyzer #4487

merged 74 commits into from
Jan 31, 2025

Conversation

jesmrec
Copy link
Collaborator

@jesmrec jesmrec commented Oct 4, 2024

Related Issues

App: #4506

  • Add changelog files for the fixed issues in folder changelog/unreleased. More info here
  • Add feature to Release Notes in ReleaseNotesViewModel.kt creating a new ReleaseNote() with String resources (if required)

QA

@jesmrec jesmrec force-pushed the feature/detekt branch 7 times, most recently from a0c1d33 to e0fe17b Compare October 9, 2024 15:32
@jesmrec jesmrec force-pushed the feature/detekt branch 2 times, most recently from 6bfce71 to 340aebb Compare October 16, 2024 07:41
@jesmrec jesmrec force-pushed the feature/detekt branch 6 times, most recently from cd5e812 to 59aeb55 Compare October 30, 2024 08:23
@jesmrec jesmrec linked an issue Oct 30, 2024 that may be closed by this pull request
48 tasks
@jesmrec jesmrec force-pushed the feature/detekt branch 3 times, most recently from 94396a2 to f939d1f Compare October 30, 2024 16:20
@jesmrec jesmrec force-pushed the feature/detekt branch 2 times, most recently from 0836d61 to b96e246 Compare November 20, 2024 10:10
@JuancaG05 JuancaG05 changed the title [FEATURE] Detekt as static code analyzer [TECHNICAL] Detekt: static code analyzer Dec 2, 2024
Copy link
Collaborator

@JuancaG05 JuancaG05 left a comment

Choose a reason for hiding this comment

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

Some comments to take into account before we merge 😃 @jesmrec

gradle/libs.versions.toml Outdated Show resolved Hide resolved
.github/workflows/detekt.yml Outdated Show resolved Hide resolved
.github/workflows/detekt.yml Outdated Show resolved Hide resolved
.github/workflows/detekt.yml Outdated Show resolved Hide resolved
.github/workflows/detekt.yml Outdated Show resolved Hide resolved
@JuancaG05
Copy link
Collaborator

JuancaG05 commented Dec 10, 2024

MaximumLineLength, from the "Formatting" section, is currently enabled. In the docs it says it overlaps with MaxLineLength, which we are relying on, and they have different values. Needs to be checked.

EDIT: "Formatting" section was not yet considered for this iteration, but we'll need to use the MaximumLineLength rule since it has the ignoreBackTickedIdentifier parameter, which allows to ignore the lines for the tests names with the backtick format, which we use and that currently exceed the line length limit (there is no way to break them in more than 1 line).

These reports will be avoided for the moment with @Suppress annotations, but once we enable MaximumLineLength they should be removed.

@JuancaG05
Copy link
Collaborator

Just checked (tested empirically) with the LongMethod rule that Detekt counts LLOC when talking about lines of code, and so I guess this happens with every rule that has to do with lines of code. That means, when trying to reduce lines of code, don't play with blank lines or comments since that won't make effect 😉

@jesmrec jesmrec force-pushed the feature/detekt branch 3 times, most recently from 8e7d781 to e61ea92 Compare January 30, 2025 16:17
@jesmrec
Copy link
Collaborator Author

jesmrec commented Jan 30, 2025

Will test:

  • Code smell in ownCloudApp
  • Code smell in ownCloudDomain
  • Code smell in ownCloudData
  • Code smell in ownCloudTestUtil
  • Code smell in ownCloudComLibrary
  • Code smell in all modules

@JuancaG05 JuancaG05 marked this pull request as ready for review January 30, 2025 16:36
@jesmrec
Copy link
Collaborator Author

jesmrec commented Jan 31, 2025

This is OK to go. This first iteration:

  • Will have a complete set of code smells fixed, but others are pending. New issue to be created

  • The execution of detekt is sequential through the app modules. If in one module are detected code smells, the execution will stop. We will check if this behaviour could be improved somehow.

@joragua joragua merged commit e90013a into master Jan 31, 2025
17 checks passed
@joragua joragua deleted the feature/detekt branch January 31, 2025 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TECHNICAL] Detekt: static code analyzer
3 participants