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.

Sign up for GitHub

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

Jump to bottom

[Update Detekt Version] Update Detekt to 1.17.1 #16928

Merged
merged 16 commits into from
Jul 22, 2022
Merged

[Update Detekt Version] Update Detekt to 1.17.1 #16928

merged 16 commits into from
Jul 22, 2022

Conversation

ParaskP7
Copy link
Contributor

@ParaskP7 ParaskP7 commented Jul 20, 2022 •
edited
Loading

Parent: #16892
Closes: #16895

This PR:

  • Updates Detekt to 1.17.1 (from 1.16.0) (a6a009c).
  • Resolved/Suppresses new warnings that started appearing due to the fact the below test sources are now included by default (previously excluded by default)
    • src/test/java
    • src/test/kotlin

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).

Merge instructions:

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.

ParaskP7 added 11 commits July 20, 2022 15:44
@ParaskP7
Build: Update detekt to 1.17.1
a6a009c 
With this update, the test sources below, which were not included by
default before, are now included by default:

- src/test/java
- src/test/kotlin

As such, for all the above test sources, Detekt has started to warn
about current violations based on the existing rules. As such, those
need to be resolved/suppressed accordingly. This is going to be done in
subsequent commits.
@ParaskP7
Analysis: Suppress long parameter list test warnings for wp module
758319f
@ParaskP7
Analysis: Suppress large class test warnings for wordpress module
f710e27
@ParaskP7
Analysis: Suppress long method test warnings for wordpress module
a1f864a
@ParaskP7
Analysis: Suppress nested block depth test warning for wordpress module
0eaedd0
@ParaskP7
Analysis: Resolve empty function block test warnings for wp module
811fd61
@ParaskP7
Analysis: Suppress swallowed exception test warning for wp module
6aa67ce
@ParaskP7
Analysis: Suppress max line length test warning for wordpress module
7d65b8c
@ParaskP7
Analysis: Resolve/Suppress unused private member test warnings for wp
3fd029f
@ParaskP7
Analysis: Resolve may be const test warnings for wordpress module
44a4223
@ParaskP7
Analysis: Suppress unnecessary abstract class test warning for wp mod.
78bf04e
@ParaskP7 ParaskP7 added this to the 20.5 milestone Jul 20, 2022
@ParaskP7 ParaskP7 requested review from irfano and a team July 20, 2022 14:18
@ParaskP7 ParaskP7 self-assigned this Jul 20, 2022
@ParaskP7 ParaskP7 changed the title Update/detekt to 1.17.1 [Update Detekt Version] Update Detekt to 1.17.1 Jul 20, 2022
@ParaskP7 ParaskP7 mentioned this pull request Jul 20, 2022
Closed
11 tasks
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 20, 2022 •
edited
Loading

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

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 20, 2022 •
edited
Loading

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

Base automatically changed from update/detekt-to-1.16.0 to trunk July 21, 2022 08:45
oguzkocer
oguzkocer approved these changes Jul 21, 2022
View reviewed changes
Copy link
Contributor

@oguzkocer oguzkocer left a comment

Choose a reason for hiding this comment

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

@ParaskP7 I left a few syntax suggestions. Looks great, thank you for this update! 🙇‍♂️

WordPress/src/test/java/org/wordpress/android/ui/reader/discover/ReaderDiscoverViewModelTest.kt Outdated Show resolved Hide resolved
...Press/src/test/java/org/wordpress/android/ui/reader/discover/ReaderPostUiStateBuilderTest.kt Outdated Show resolved Hide resolved
WordPress/src/test/java/org/wordpress/android/ui/stats/refresh/utils/StatsSiteProviderTest.kt Outdated Show resolved Hide resolved
WordPress/src/test/java/org/wordpress/android/ui/stats/refresh/utils/StatsSiteProviderTest.kt Outdated Show resolved Hide resolved
@ParaskP7
Copy link
Contributor Author

@ParaskP7 I left a few syntax suggestions. Looks great, thank you for this update! 🙇‍♂️

Thank you so much for the review and the 👍 @oguzkocer , I applied all your syntax suggestions! 🙇 ❤️ 🚀

@ParaskP7 ParaskP7 added Tooling and removed Tooling labels Jul 22, 2022
@ParaskP7 ParaskP7 merged commit 104f31c into trunk Jul 22, 2022
@ParaskP7 ParaskP7 deleted the update/detekt-to-1.17.1 branch July 22, 2022 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oguzkocer oguzkocer oguzkocer approved these changes

@irfano irfano Awaiting requested review from irfano

Assignees

@ParaskP7 ParaskP7

Labels
dependency update Tech Debt Tooling
Projects
None yet
Milestone
20.5
Development

Successfully merging this pull request may close these issues.

Update Detekt Version - Update to 1.17.1
3 participants
@ParaskP7 @wpmobilebot @oguzkocer