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

Load all rules from QP at once #984

Merged
merged 7 commits into from
Apr 1, 2021

Conversation

pavel-mikula-sonarsource
Copy link
Contributor

@pavel-mikula-sonarsource pavel-mikula-sonarsource commented Mar 31, 2021

This is preparation for #980

Old state:

  • Load active rules.
  • Load inactive rules (second request to the server API).
  • Propagate two sets of rules
  • Merge the lists and process it.

New state:

  • Load all rules.
  • Propagate single set.
  • Process it.

@pavel-mikula-sonarsource
Copy link
Contributor Author

There's ongoing AZP incident, the IT job failed on a cache task.

Copy link
Contributor

@mickael-caro-sonarsource mickael-caro-sonarsource left a comment

Choose a reason for hiding this comment

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

Looks good !

Copy link
Member

Choose a reason for hiding this comment

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

LGTM. Just a few minor suggestions.

}
if (r["templateKey"] != null)
var active = actives?.Value<JArray>(r["key"].ToString())?.FirstOrDefault();
var rule = new SonarRule(r["repo"].ToString(), ParseRuleKey(r["key"].ToString()), r["internalKey"]?.ToString(), r["templateKey"]?.ToString(), active != null);

Choose a reason for hiding this comment

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

Would be great to extract the strings as constants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used just once.

Choose a reason for hiding this comment

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

There are some which are used multiple times e.g. "key"

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 1, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

96.0% 96.0% Coverage
0.0% 0.0% Duplication

@pavel-mikula-sonarsource pavel-mikula-sonarsource merged commit 71c47d4 into branch-5.2 Apr 1, 2021
@pavel-mikula-sonarsource pavel-mikula-sonarsource deleted the Pavel/RefactorRuleProcessing branch April 1, 2021 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants