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

[Update Detekt Version] Update Detekt to 1.19.0 #16949

Merged
merged 5 commits into from
Jul 27, 2022
Merged

Conversation

ParaskP7
Copy link
Contributor

@ParaskP7 ParaskP7 commented Jul 26, 2022

Parent: #16892
Closes: #16896
Closes: #16897

This PR:

PS: I recommend reviewing this PR commit-by-commit.


To test:

  • There is nothing much to test here.
  • Verifying that all the CI checks are successful should be enough (especially the detekt check).

Regression Notes

  1. Potential unintended areas of impact

N/A

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

See To test section above.

  1. What automated tests I added (or what prevented me from doing so)

N/A


PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 26, 2022

You can test the WordPress changes on this Pull Request by downloading an installable build (wordpress-installable-build-pr16949-65cc613.apk), or scanning this QR code:

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 26, 2022

You can test the Jetpack changes on this Pull Request by downloading an installable build (jetpack-installable-build-pr16949-65cc613.apk), or scanning this QR code:

ParaskP7 added 4 commits July 26, 2022 17:03
With this update, the configuration for the rules below got
updated. Also, one rule, specific to Jetpack Compose, was added:

Rules that got updated:
- LongParameterList (complexity)
- ForbiddenComment (style)

Rules that got added:
- FunctionNaming (naming)

As such, for all the above listed rules, Detekt has started to warn
about current violations based on these rule updates. As such those
rules need to be updated/added accordingly. This is going to be done in
subsequent commits.
With the '1.19.0' Detekt update, the Detekt 'ignoreAnnotated' feature
got updated. With that update, the 'javax.inject.Inject' configuration
was no longer working and had to be update to its simpler 'Inject'
version, which doesn't require the full import naming convention.

Also, as part of this update the 'javax.annotation.Generated' or
'Generated' configuration got removed as it is no longer (or was never)
necessary. Thus, making this overall configuration much more
straightforward.
With the '1.19.0' Detekt update, the Detekt 'values' configuration
got updated. With that update, the '['FIXME:', 'STOPSHIP:']' map,
although it keeps working as expected, can be update to the list
equivalent configuration, which also matches to the
'default-detekt-config.yml' file's configuration.

Default Detekt Config: https://github.com/detekt/detekt/blob/main/
detekt-core/src/main/resources/default-detekt-config.yml
This is done in order to ignore the 'Composable' related warning for
the 'WordPressTheme' function naming convention.

For more info see: https://detekt.dev/docs/introduction/compose
@ParaskP7 ParaskP7 force-pushed the update/detekt-to-1.19.0 branch from d993076 to 65cc613 Compare July 26, 2022 14:03
@ParaskP7
Copy link
Contributor Author

FYI: We can ignore the Buildkite related Instrumented Tests check failing for now. I can merge this to trunk anyway since I think those test are just being flaky and are not related to the Detekt update anyhow (see Firebase Test Lab).

Cc @jkmassel @pachlava

@ParaskP7 ParaskP7 merged commit 1f5f178 into trunk Jul 27, 2022
@ParaskP7 ParaskP7 deleted the update/detekt-to-1.19.0 branch July 27, 2022 08:25
Copy link
Contributor

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

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

Awesome work @ParaskP7!
Thanks a lot for it 🙇 🥇 💯

Now it should theoretically be possible for me to replace UnusedPrivateMember. allowedNames.regex with UnusedPrivateMember. ignoreAnnotated[Preview] ❤️ in the pending Compose PR.

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.

Update Detekt Version - Update to 1.19.0 Update Detekt Version - Update to 1.18.1
4 participants