MeltanoLabs / Meta

The why, what, and how of MeltanoLabs
MIT License
5 stars 1 forks source link

Need to ignore "code duplication" failure in SonarCloud #13

Open aaronsteers opened 2 years ago

aaronsteers commented 2 years ago

@pnadolny13 - We're getting a failure related to code duplication in this PR: https://github.com/MeltanoLabs/tap-github/pull/93/checks

SonarCloud page:

Code duplication is expected here because each stream has its own schema. Although the source APIs overlap a lot, we don't want to refactor the entire stream definitions to have to read and maintain the schemas off of multiple files. (We also can't guarantee that the source API will change all in tandem when one is changed.)

aaronsteers commented 2 years ago

I'm new to SonarCloud and not sure what kind of "#noqa" options we have. Ideally, we'd either disable the "code duplication" check org-wide (since it's likely to also trigger on other repos for the same reason), or find a way to ignore just in this project.

aaronsteers commented 2 years ago

@pnadolny13, @edgarrmondragon, @tayloramurphy (+ @pandemicsyn in case you have insights 😄 )

This was quite a headache for us in https://github.com/MeltanoLabs/tap-github/pull/93. Does anyone know if we can disable this check?

Code duplication makes sense to check for in other projects, but I don't think it makes sense for taps. The stream schemas are very likely to trigger code duplication errors, which we should always ignore IMHO. The author of the tap should publish the schema in whichever way is most efficient to maintain. If this means copy-pasting from the vendor's website we should support that. If it means defining each stream explicitly in Python, we should support that. I don't think we can infer that a stream's schema overlapping presently with another stream in the API is necessarily a guarantee that they will always overlap or that the overlap is sufficient to warrant refactoring.

So, I see three paths forward (really two):

  1. We disable sonarcloud altogether, which I don't this is warranted.
  2. We find a way to disable just the code duplication checks (if possible).
  3. We add some process documentation on how to manage code duplication alarms in community MRs. For instance: the community maintainer (Ovio in this case) should resolve all other CI issues and open PR threads, then ping Meltano for to force-merge with the "Administrator" option.

What do you all think?

Ideally I'd like to go with option 2 - but if that's not possible I think option 3 would work.

aaronsteers commented 2 years ago

Trying a ping to the community forums here: https://community.sonarsource.com/t/how-to-fully-ignore-code-duplication-for-a-project-or-org/61326?u=aaronsteers

edgarrmondragon commented 2 years ago

@aaronsteers we got a solution from the Sonar folks: https://community.sonarsource.com/t/how-to-fully-ignore-code-duplication-for-a-project-or-org/61326/2

sonar.cpd.exclusions=**/*

I'll start a PR in the tap-github to test it.