Detekt - Resolve/Suppress All Baseline Warnings

[ParaskP7]

Parent #16892

This issue is about resolving and/or suppressing any baseline Detekt warnings on this repo.

FYI: There are currently about 291 Detekt related warnings within the baseline.xml file (give-or-take).

[peril-wordpress-mobile]

	Fails
🚫	Please add a type label to this issue. e.g. '[Type] Enhancement'

Generated by 🚫 dangerJS

[peril-wordpress-mobile]

	Fails
🚫	Please add a type label to this issue. e.g. '[Type] Enhancement'

Generated by 🚫 dangerJS

[peril-wordpress-mobile]

	Fails
🚫	Please add a type label to this issue. e.g. '[Type] Enhancement'

Generated by 🚫 dangerJS

[hakeem-gitstart]

@ParaskP7 if you don't mind, can you give more insight on what to do here, i'm not very conversant with detekt, but will like to pick up this issue

[ParaskP7]

👋 @hakeem-gitstart and thank you for wanting to pick-up this issue! 🥇

i'm not very conversant with detekt

Not to worry about that, I would first suggest you taking a look at Detekt, read the docs and more specifically the Rules Documentations, like the Style Rule Set. There are 10 such rule documentation pages and all of them provide you with information about what these rule do and why, along with an example of a non-compliant code vs a compliant code (see UseCheckOrError as an example). If you are already comfortable with Kotlin, all this should be pretty straightforward for you.

can you give more insight on what to do here

For sure! 💯 PS: You can start by taking a look at this issue that I am currently working on, which does exactly that but for the fluxc module for WordPress-FluxC-Android. You will need to do the same for the WordPress module in this repo. The rest of the modules on this repo are baseline free already.

So, on the WordPress-FluxC-Android, what I did was:

• To start with checking its baseline.xml file, focusing on the <!-- fluxc --> module section single this issue is about this module alone. Note that you don't have to search for any such module section for the baseline.xml in this repo as all the issues there are for the WordPress module anyway.
• Then, for every issue on this baseline, I noted it, its rule set category and counted its occurrence (for example, the ComplexCondition, for the complexity rule set, with 6 occurrences). You will ended up with having all issues counted and grouped per rule set category.
• At this point, you should split your work as you sit fit. You don't want to resolve/suppress all issues in a single PR as this won't be ideal, neither for you, nor the reviewer. Thus, and depending on the count and complexity of the issues you will end up with, try to create as many tasks as you see fit into a single PR. PS: That's what I did here.
• Then, start with the first task and the first rule on that task, one-by-one, and try to figure out if this warning can be resolved or suppressed:
  • If this warning can be resolved, remove it from the baseline, apply the fix and progress. If the fix is quite complex or there are no more such warnings, commit this change straight away. If the fix is quite simple and there are more of such warnings, you might want to gather all such changes into a single commit.
  • If this warning can't or shouldn't be resolved, remove it from the baseline and suppress this warning inline and as close to the source as possible. If there are no more such warnings that need to be suppressed, commit this changes straight away. If there are more such warnings that need to be suppressed, you might want to gather all such suppressions into a single commit.

I can go into more detail about the process I am following while working on such issues, but I think the above would give you an initial understanding of what needs to be done here. Wdyt?

Let me know if all that helps! 🌟

[hakeem-gitstart]

@ParaskP7 thanks, will reach out if there's any issue

[ParaskP7]

Awesome, for anything you might need going forward I am here to support you @hakeem-gitstart ! 🥇

[ParaskP7]

👋 again @hakeem-gitstart with a quick FYI from my side. I just created an accompanying PR, corresponding to this issue I shared with you above. Maybe it will be of help to you to understand the process I suggested on how to resolve/suppress the WPAndroid related Detekt baseline warnings, one PR and one commit at a time.

[hakeem-gitstart]

@ParaskP7 this is really great and do put things in perspective clearly, thank you very much. i will put up a PR tomorrow

[hakeem-gitstart]

hey man @ParaskP7, kindly check your email when you're chanced

[ParaskP7]

👋 @hakeem-gitstart thank you for working on this issue and for sending me this personal email, with the video link and all, much appreciated! 🌟

To top that up, may I suggest this one basic tips for you: Please don't feel you need to do all that privately, what you shared with me in that personal email, along with the short video link, could and can be easily shared here as well, effectively doing that in public, for everyone to see and benefit from. After all, sharing is caring! 🧑‍🏭 🧑‍🏭

Now, as far the work you've done so far, I recommend we do it step-by-step in order for you to avoid doing extra work that might need much refinement afterwards (due to various reasons). To that end, I recommend you to create a PR, just like you did on the video and send it our way. But, this time, try to focus on solving one specific warnings, for example the MagicNumber warnings alone, just like you did in that PR you shared, excluding the MaxLineLength warning fixes and separating them from this first PR of yours.

I am suggesting the above because, although you seeing me fixing multiple kind of warnings, or even multiple category of warnings per PR(see here), this doesn't mean that you can't take it slow. You actually can and I recommend you do. Then, and after we refine your first PR together, like its title, description, commits and file changes, you will then have a better idea of what to expect from us going forward (and vice-versa). Then, on your next PR, we can discuss about it again and up your level to solve multiple warnings. Wdyt?

[hakeem-gitstart]

@ParaskP7 sure, sounds great 👍 , will create a different PR addressing just one of the issue

[hakeem-gitstart]

@ParaskP7 can you kindly check this PR , it's been up for a while now. let me know if there's anything i'm missing . thanks

[ParaskP7]

👋 @hakeem-gitstart and thank you for opening the PR! 🎉

I am currently on vacation and thus I won't be able to help you much. However, I am returning in about 10 days. I'll make sure to take a look on my return.