feat(analyze): Add DCM and fix potentiall bugs, depreciations

[mhadaily]

Description

This pull request introduces several changes, primarily focusing on integrating Dart Code Metrics (DCM) into the CI workflow, updating the contributing guidelines, and making various code improvements and refactorings. The most important changes are summarized below:

CI/CD Workflow Enhancements:

• Added a step to install DCM in the GitHub Actions workflow (.github/workflows/checks.yml).
• Updated the analysis step to use DCM for both Flutter and Dart in the GitHub Actions workflow (.github/workflows/checks.yml).

Documentation Updates:

• Updated CONTRIBUTING.md to include steps for installing and activating DCM, and clarified the use of DCM in the contribution process.

Code Quality and Linting:

• Added Dart Code Metrics configuration to analysis_options.yaml and other related files to enforce specific linting rules. [1] [2] [3]

Code Refactoring:

• Refactored multiple files to improve code readability and maintainability, including changes to widget properties and method calls. [1] [2] [3] [4] [5] [6] [7]

Minor Code Improvements:

• Made minor adjustments to formatting and method calls to enhance code consistency and readability. [1] [2] [3] [4]

These changes collectively improve the project's CI/CD pipeline, documentation, code quality, and overall maintainability.

Related Issues

Replace this paragraph with a list of issues related to this PR from the issue database. Indicate, which of these issues are resolved or fixed by this PR.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• My PR includes tests for all changed/updated/fixed behaviors.
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (melos run analyze) does not report any problems on my PR.
• The package compiles with the minimum Flutter version stated in the pubspec.yaml

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change.
• No, this is not a breaking change.

[ulusoyca]

@mhadaily Can we have an example here to demonstrate how to create an internal static analysis package that includes all the rules and is also added as dependency to the demo apps and also to the package lib? I see that currently every demo app has its own analysis option. We can add baseline to silence warnings in the demo apps if they are too big changes.

[ulusoyca]

We can accept all the suggsted changes in the demo app. My comments are focusing only the package lib. I left minor comments. Huge thanks 🚀

[ulusoyca]

@mhadaily Looks like the workflow is expecting an approval to run. Is this intentional? If yes, it does not sound useful since we expect the static analysis feedback from CI to work even in draft PRs.

[incendial]

@mhadaily Looks like the workflow is expecting an approval to run. Is this intentional? If yes, it does not sound useful since we expect the static analysis feedback from CI to work even in draft PRs.

Hmm, if that's related to this PR, then that's probably because of how GitHub actions work. Either DCM needs to be installed without the action or another trigger should be used (example: https://github.com/leoafarias/fvm/blob/main/.github/workflows/test.yml#L45).

I'd say the first approach is safer.

[mhadaily]

@mhadaily Looks like the workflow is expecting an approval to run. Is this intentional? If yes, it does not sound useful since we expect the static analysis feedback from CI to work even in draft PRs.

@ulusoyca according to this https://docs.github.com/en/actions/managing-workflow-runs-and-deployments/managing-workflow-runs/approving-workflow-runs-from-public-forks
It seems since I am first time contributor and I am touching some pull_request_target, you need to approve this change and run.

it looks like this is only for first time contributor and will not happen later. Let's give it a try.

[mhadaily]

We can accept all the suggsted changes in the demo app. My comments are focusing only the package lib. I left minor comments. Huge thanks 🚀

Awesome, first two comments.

[ulusoyca]

@mhadaily WidgetStatePropertyAll fill fail because the min Flutter SDK version is 3.13.0+

[mhadaily]

@mhadaily WidgetStatePropertyAll fill fail because the min Flutter SDK version is 3.13.0+

AH that's right, I didn't check that. Should I change min SDK or you prefer I revert the change for "WidgetStatePropertyAll" ?

[ulusoyca]

@mhadaily WidgetStatePropertyAll fill fail because the min Flutter SDK version is 3.13.0+

AH that's right, I didn't check that. Should I change min SDK or you prefer I revert the change for "WidgetStatePropertyAll" ?

Actually it is a good idea to change the min SDK in the example apps, without changing the package min SDK constraint.

[mhadaily]

Done @ulusoyca

[ulusoyca]

🚢

[mhadaily]

btw, @ulusoyca when you got chance, try to enable the "disabled rule" and fix the issue, I find several potential bugs and also memory leaks problems.

also if you run dcm check-unused-files and dcm check-unused-files you will see you also have dead code and files.

generally a good idea to play around with some of the commands

[ulusoyca]

@mhadaily Thanks for the setup, I will delete / simplify some demo projects, and add / enable more rules

[mhadaily]

@ulusoyca It seems the main library actually had 3.13 but the examples were not. so I changed everything, Not sure why it needs approval again. it was working without approval but suddenly last commit needs it again.

[ulusoyca]

🚢

[mhadaily]

It's still failing, I am not sure why? do you have any idea?
I checked it on my machine and it seems it's all good

[ulusoyca]

It's still failing, I am not sure why? do you have any idea? I checked it on my machine and it seems it's all good

Your Local is probably the latest flutter but the action is run on 3.13.0. I think the examples should use MaterialState instead of WidgetState

[mhadaily]

Well, that is correct, I thought WidgetState is available after 3.13, anyway I have revert it to MaterialState. Maybe approve again and let's see how it works.

if it passes then we are good. I can probably deal with depreciation when you are ready.

[mhadaily]

@ulusoyca sorry again 😄

one more time approval. I think this is the final time. Though tests are failing and I think you know that already

[ulusoyca]

@mhadaily we are good now!

[ulusoyca]

@mhadaily can we merge now?

[mhadaily]

Yes please *Best regards, * Majid

…

On Fri, 6 Sep 2024 at 09:54, Cagatay Ulusoy ***@***.***> wrote: @mhadaily <https://github.com/mhadaily> can we merge now? — Reply to this email directly, view it on GitHub <#315 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKDTC3J6NNQGU2QILIDLBTZVFNSTAVCNFSM6AAAAABNEJGNP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZTGQ3DIMRTGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>