dart-code-checker / dart-code-metrics

Software analytics tool that helps developers analyse and improve software quality.
https://dcm.dev
Other
860 stars 266 forks source link

[Question] use the package with melos #395

Closed imtoori closed 3 years ago

imtoori commented 3 years ago

I'm trying to use the package in a monorepo with several packages We're using melos https://github.com/invertase/melos with a single analysis_options.yaml file in the root of the repo The configuration in the ide is working fine, but I'm struggling to make it work in the cli

it seems like it's not taking the configuration options from the analysis_options file

Is there anything I should configure? Let me know if you need more info

dkrutskikh commented 3 years ago

@imtoori thank you for interest in our project. Can you provide a minimal representative example?

incendial commented 3 years ago

it seems like it's not taking the configuration options from the analysis_options file

Could you please share the config, the expected result and the outcome you get?

imtoori commented 3 years ago

https://github.com/GetStream/stream-chat-flutter/tree/feat/dart-metrics

I pushed the configuration I'm trying in this branch as a WIP commit to show what I'm trying to do

I'm adding the plugin in the top-level analysis_options.yaml

I added a melos script called metrics that runs

flutter pub run dart_code_metrics:metrics lib --set-exit-on-violation-level=warning

in every package

The output shows warnings and alarms that I haven't configured For example it shows number-of-methods warnings, but that's not present in my analysis_options file

Maybe we need a way to specify a config file in the cli options?

incendial commented 3 years ago

Thank you for the example, we'll take a closer look and return with a possible solution.

incendial commented 3 years ago

@imtoori sorry for the delay, we found two problems so far:

  1. Cli is not picking up the config from the root of the repo and there is no way to pass it. It works different for the plugin which explains why the plugin is working correctly.
  2. There is no way to disable metrics, only to set up their thresholds. That works as initially intended, but now it seems inflexible and should be changed. So a possible work-around would be to set up high thresholds for metrics you don't want to see reported (but it's a temporary solution before we redesign metrics calculation). But the problem with config pick up will still remain.

Could you also share the motivation why you disable certain metrics? Are they useless for you or there is another reason?

imtoori commented 3 years ago

ok it seems like it's not supposed to be used with melos using a single analysis_options 😢

I'm disabling certain metrics mostly because we're trying to enforce them gradually and not everything at once

thanks for looking into this btw 🙂

incendial commented 3 years ago

ok it seems like it's not supposed to be used with melos using a single analysis_options 😢

Does it mean that it's a blocker for you to start using cli?

I'm disabling certain metrics mostly because we're trying to enforce them gradually and not everything at once

Hm, so will the approach to start with high thresholds actually work for you?

thanks for looking into this btw 🙂

No problem, we are really excited that you interested in our project and I hope we can resolve all issues.

imtoori commented 3 years ago

Hm, so will the approach to start with high thresholds actually work for you?

yes I think this should work

Does it mean that it's a blocker for you to start using cli?

I think so, we're trying to enforce the rules in our github actions

incendial commented 3 years ago

@imtoori think we have an idea how to support your case with a single analysis_options file correctly, hope we'll release it soon. Not closing this issue till we release it.

incendial commented 3 years ago

@imtoori we published 4.0.2-dev.1 version which should work correctly with melos and a single analysis options file. Could you check it out and tell if it works as you expected?

incendial commented 3 years ago

@imtoori sorry for pinging, but we really want to know, that 4.0.2-dev.1 version fixes your problem before we can release it as 4.0.2. Any chances you can take a look at it?

imtoori commented 3 years ago

Hi @incendial I'll try it today and let you know if that works! Thanks again!

imtoori commented 3 years ago

@incendial that seems to work! Thanks a lot!

incendial commented 3 years ago

@imtoori great! 🎉 🎉 🎉 Thanks a lot for checking! jfyi, we'll release a stable 4.0.2 after a small fix for a check unused files command.

incendial commented 3 years ago

Ended up releasing it as 4.1.0 since it looks more like a feature.