-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[sonar scan] Scan public directories #190350
Conversation
legrego helpfully pointed out that these files should be scanned - they include client side code. The replaces the exlucsion for public with target, the compiled version of this source.
Pinging @elastic/kibana-operations (Team:Operations) |
sonar-project.properties
Outdated
@@ -38,7 +38,7 @@ sonar.exclusions=\ | |||
**/mock_responses/**/*, \ | |||
**/mocks/**/*, \ | |||
**/node_modules/**/*, \ | |||
**/public/**/*, \ | |||
**/target/**/*, \ | |||
**/scripts/**/*, \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly related, but **/scripts/**/*
is a bit too broad. It excludes valid source directories such as src/plugins/data/server/scripts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I attempted to enumerate the script directories that I don't care about in https://github.com/elastic/kibana/pull/190347/files. I can't guarantee it's correct, so use at your own risk 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with 09cbbc3 in case new folders are added. I didn't include the root level scripts
directory because it's not included initially. Seem reasonable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with 09cbbc3 in case new folders are added. I didn't include the root level
scripts
directory because it's not included initially. Seem reasonable?
I'm surprised that **/scripts/**/*
wouldn't match the root level scripts
, but if that's how it behaved before, then what you have seems fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with 09cbbc3 in case new folders are added. I didn't include the root level
scripts
directory because it's not included initially. Seem reasonable?I'm surprised that
**/scripts/**/*
wouldn't match the root levelscripts
, but if that's how it behaved before, then what you have seems fine to me.
To clarify, it would match it but we're not including it in folder sources list
kibana/sonar-project.properties
Lines 5 to 9 in bff5500
sonar.sources=\ | |
packages, \ | |
src, \ | |
x-pack/packages, \ | |
x-pack/plugins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah my mistake. LGTM!
legrego helpfully pointed out that these files should be scanned - they include client side code. The replaces the exclusion of `public` with `target`, the compiled version of this source.
legrego helpfully pointed out that these files should be scanned - they include client side code. The replaces the exclusion of
public
withtarget
, the compiled version of this source.