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.

Sign up for GitHub

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

Jump to bottom

fix: add collect sonarqube mode by file #6507

Merged
merged 7 commits into from
Nov 28, 2023
Merged

fix: add collect sonarqube mode by file #6507

merged 7 commits into from
Nov 28, 2023

Conversation

abeizn
Copy link
Contributor

@abeizn abeizn commented Nov 23, 2023 •
edited
Loading

Summary

fix: add collect sonarqube mode by file

Does this close any open issues?

Closes #6493

Screenshots

image

Other Information

Any other information that is important to this PR.

leric
leric reviewed Nov 23, 2023
View reviewed changes
backend/plugins/sonarqube/tasks/issues_collector.go Outdated
logger.Info("split by dir for it's count:[%d] and val:[%s]", value.Count, value.Val)
} else {
// by file
for _, issue := range body.Issues {

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

@abeizn abeizn requested a review from klesh November 24, 2023 06:21
leric
leric reviewed Nov 27, 2023
View reviewed changes
backend/plugins/sonarqube/tasks/issues_collector.go Outdated
if value2.Count > MAXISSUECOUNT {
logger.Warn(fmt.Errorf("the issue count [%d] exceeds the maximum page size", value2.Count), "")
} else {
iterator.Push(&SonarqubeIssueIteratorNode{

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

d4x1
d4x1 reviewed Nov 28, 2023
View reviewed changes
backend/plugins/sonarqube/tasks/issues_collector.go Outdated
logger.Info("split by dir, and it's issue count:[%d] and file path:[%s]", value.Count, value.Val)
} else {
// split it by dir when it's issue count > 10000
res2, err := data.ApiClient.Get("issues/search", url.Values{
Copy link
Contributor

Choose a reason for hiding this comment

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

res2 and body2 values2 should have better names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

backend/plugins/sonarqube/tasks/issues_collector.go Outdated
CreatedBefore: createdBefore,
FilePath: value2.Val,
})
logger.Info(fmt.Sprintf("split by fil, and it's issue count:[%d] and file path:[%s]", value2.Count, value2.Val))
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to use fmt.Sprintf. logger.Info has a format parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

backend/plugins/sonarqube/tasks/issues_collector.go

type SonarqubePageInfo struct {
Total int `json:"total"`
P int `json:"p"`
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the meaning of p and ps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

p: 1-based page number
ps: Page size. Must be greater than 0 and less or equal than 500

backend/plugins/sonarqube/tasks/issues_collector.go
}
for _, value2 := range body2.Facets[0].Values {
if value2.Count > MAXISSUECOUNT {
logger.Warn(fmt.Errorf("the issue count [%d] exceeds the maximum page size", value2.Count), "")
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to use fmt.Errorf.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

func (log.Logger).Warn(err error, format string, a ...interface{})
The first parameter is of error type, so it is required

backend/plugins/sonarqube/tasks/issues_collector.go
"componentKeys": {fmt.Sprintf("%v", data.Options.ProjectKey)},
"directories": {value.Val},
"facets": {"files"},
"ps": {"1"},
Copy link
Contributor

@d4x1 d4x1 Nov 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can this work? I think it should be "ps": []string{"1"}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes,
"ps": {"1"} is correct
"ps": {}string{"1"} is invalid syntax

klesh
klesh approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@klesh klesh left a comment

Choose a reason for hiding this comment

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

LGTM

@abeizn abeizn merged commit bffcbcb into main Nov 28, 2023
10 checks passed
@abeizn abeizn deleted the fix#6316-byfile branch November 28, 2023 05:29
github-actions bot pushed a commit that referenced this pull request Nov 28, 2023
@abeizn @d4x1
fix: add collect sonarqube mode by file (#6507)
4e869ab 
* fix: add collect sonarqube mode by file

* fix: update some collector logic

* fix: some params

* fix: update some params

* fix: still need to push issues when the count exceed MAXISSUECOUNT

* fix: update log format and variable name

---------

Co-authored-by: Lynwee <linwei.hou@merico.dev>
@github-actions github-actions bot mentioned this pull request Nov 28, 2023
Merged
@github-actions GitHub Actions
Copy link

🤖 cherry pick finished successfully 🎉!

@github-actions github-actions bot added the bot/auto-cherry-pick-completed auto cherry pick completed label Nov 28, 2023
abeizn added a commit that referenced this pull request Nov 29, 2023
@github-actions @abeizn @d4x1
fix: add collect sonarqube mode by file (#6507) (#6531)
e61a8c8 
* fix: add collect sonarqube mode by file

* fix: update some collector logic

* fix: some params

* fix: update some params

* fix: still need to push issues when the count exceed MAXISSUECOUNT

* fix: update log format and variable name

---------

Co-authored-by: abeizn <zikuan.an@merico.dev>
Co-authored-by: Lynwee <linwei.hou@merico.dev>
github-actions bot pushed a commit that referenced this pull request Nov 29, 2023
@abeizn @d4x1
fix: add collect sonarqube mode by file (#6507)
25c1b51 
* fix: add collect sonarqube mode by file

* fix: update some collector logic

* fix: some params

* fix: update some params

* fix: still need to push issues when the count exceed MAXISSUECOUNT

* fix: update log format and variable name

---------

Co-authored-by: Lynwee <linwei.hou@merico.dev>
@github-actions github-actions bot mentioned this pull request Nov 29, 2023
Merged
@github-actions GitHub Actions
Copy link

🤖 cherry pick finished successfully 🎉!

@github-actions github-actions bot added the bot/auto-cherry-pick-completed auto cherry pick completed label Nov 29, 2023
abeizn added a commit that referenced this pull request Nov 29, 2023
@github-actions @abeizn @d4x1
fix: add collect sonarqube mode by file (#6507) (#6532)
970b8e3 
* fix: add collect sonarqube mode by file

* fix: update some collector logic

* fix: some params

* fix: update some params

* fix: still need to push issues when the count exceed MAXISSUECOUNT

* fix: update log format and variable name

---------

Co-authored-by: abeizn <zikuan.an@merico.dev>
Co-authored-by: Lynwee <linwei.hou@merico.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d4x1 d4x1 d4x1 left review comments

@klesh klesh klesh approved these changes

@leric leric leric approved these changes

Assignees
No one assigned
Labels
bot/auto-cherry-pick-completed auto cherry pick completed needs-cherrypick-v0.20
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature][SonarQube] Get issues by files and line numbers to pass the result limit of 10,000 records
5 participants
@abeizn @klesh @leric @d4x1 @Startrekzky