Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude code from SonarCloud analysis #2809

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

tpantelis
Copy link
Contributor

  • Generated code
  • Test support
  • External library wrappers

- Generated code
- Test support
- External library wrappers

Signed-off-by: Tom Pantelis <[email protected]>
@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr2809/tpantelis/sonar_exclude
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@tpantelis tpantelis requested a review from dfarrell07 November 22, 2023 14:05
@tpantelis tpantelis enabled auto-merge (rebase) November 22, 2023 17:52
@tpantelis tpantelis added the ready-to-test When a PR is ready for full E2E testing label Nov 22, 2023
@@ -2,7 +2,7 @@ sonar.projectKey=submariner-io_submariner
sonar.projectName=submariner
sonar.organization=submariner-io
sonar.sources=.
sonar.exclusions=**/vendor/**,**/*_test.go,**/test/**,**/fake/**,**/client/**
sonar.exclusions=**/vendor/**,**/*_test.go,**/test/**,**/fake/**,**/client/**,**/natdiscovery/proto/natdiscovery.pb.go,**/netlink/**,**/event/testing/**,**/zz_generated.deepcopy.go,**/ipset/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the removal of ipset and netlink packages intentional?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, yes, because they are external library wrappers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all APIs are passthrough, we have some APIs implemented. Do we want to avoid sonar on them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

netlink is a straight pass-through. ipset is not our code - we copied it from somewhere. There might be some helper methods we could unit test but I don't think it's necessary. I think the E2E tests it sufficiently, at least the code we use. @sridhargaddam What API are you specifically concerned about?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, FWICT, virtually all helpers, eg validate*, are private.

Copy link
Member

@sridhargaddam sridhargaddam Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, there are few APIs like Create/Add/Del/Destroy etc, but the code is pretty much stable and has not undergone any major changes, so we can skip it. I wanted to confirm that we did not accidentally remove them.

@tpantelis tpantelis merged commit 690ded7 into submariner-io:devel Nov 23, 2023
38 of 39 checks passed
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr2809/tpantelis/sonar_exclude]

@tpantelis tpantelis deleted the sonar_exclude branch December 8, 2023 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-test When a PR is ready for full E2E testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants