ci: Deepsource replacement?

preschian commented 1 year ago

seems like Deepsource was not able to read vue typescript properly. maybe replace it with sonar? https://www.sonarsource.com/products/sonarcloud/

maybe we can drop codeclimate also with sonar

cc @yangwao

yangwao commented 1 year ago

I can have look later

daiagi commented 1 year ago

I vote Aye. I opened a ticket with them about a month ago about this issue so far they didn't fix it

yangwao commented 1 year ago

let me check

it validates against package.json or somehow other way?

lets go with 10days?

so we can disable deepsource I guess

yangwao commented 1 year ago

preschian commented 12 months ago

hi @kodadot/internal-dev

Since reviewing this PR https://github.com/kodadot/nft-gallery/pull/5765#issuecomment-1532322967, I suspect that codeclimate cannot scan vue properly. So, I try to create a reproducible repo to demonstrate that. https://github.com/preschian/test-codeclimate

1. Single File Component

Demo: https://github.com/preschian/test-codeclimate/pull/1

Codeclimate is unable to scan SFC completely. All greens

2. Composables

Demo: https://github.com/preschian/test-codeclimate/pull/2

I'm good with these errors:

Similar blocks of code found in 2 locations. Consider refactoring.
Avoid too many return statements within this function.

But, for Cognitive Complexity and the number of lines, IMO not precise:

Function default has a Cognitive Complexity of 24 (exceeds 5 allowed). Consider refactoring.
Function default has 46 lines of code (exceeds 25 allowed). Consider refactoring.

Let's compare it with the Options API.

3. Options API

Demo: https://github.com/preschian/test-codeclimate/pull/3

I think this is the accurate result. Error from this PR:

Function first has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Function second has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.

In this PR, I don't get this error:

Function default has 46 lines of code (exceeds 25 allowed). Consider refactoring.

Summary

So that you know, the method to compare is the same. But, we can see the different results between Options and Composition for complexity and number of lines. IMO, it is better to turn off complexity for now or ignore the composables folder for codeclimate

Turns out, when I try to integrate my repository with codeclimate, I got this warning

daiagi commented 12 months ago

But, for Cognitive Complexity and the number of lines, IMO not precise:

100% agree.

IMO, it is better to turn off complexity

agree, it is rare that I refactor based on this warning, and for the most part it complains about functions that humans will find simple and straight forward

maybe replace it with sonar

so far I like sonar better. plus it has VSCode integration so we don't have to push the PR in order to see what issues it has

Jarsen136 commented 12 months ago

IMO, it is better to turn off complexity

In this way, we could also reduce some unnecessary refactoring work on it.

ref: https://github.com/kodadot/nft-gallery/issues/6782

preschian commented 3 months ago

Hi, can we turn off codeclimate? For example, in this PR: https://github.com/kodadot/nft-gallery/pull/10114, most of the issues are false alarms. There is no need to fix the issues with codeclimate in that PR, right?

preschian commented 3 weeks ago

hi, can we drop codeclimate please. quite annoying imo

vikiival commented 3 weeks ago

Screenshot 2024-07-26 at 14 37 04

kodadot / nft-gallery