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

Submit Summary Note before issue comments #793

Conversation

alexintech
Copy link

  • Added an option to submit Summary Note before issue comments. It can be more convenient with this PR. In that case SonarQube's Summary note will be the first thread in MR, and you don't need to scroll to the end to see results. And this PR will edit existing Summary Note, so it will be on the top of a MR.
  • Added tests for the current behavior;
  • GitlabMergeRequestDecoratorTest -> switch to JUnit 5;
  • Some efforts to simplify creation of mocks for GitlabMergeRequestDecoratorTest and minimize duplication.

Copy link
Owner

@mc1arke mc1arke left a comment

Choose a reason for hiding this comment

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

Could you alter your commit message to explain the reason for this change, i.e. include a variant of the background of In that case SonarQube's Summary note will be the first thread in MR, and you don't need to scroll to the end to see results in the commit message body, as well as any other details about other noteable points of the commit Also updated GitlabMergeRequestDecoratorTest to JUnit 5, and pulled duplicated test code into a reusable utility class

Comment on lines +161 to +169
context.addExtensions(PropertyDefinition.builder(PR_SUMMARY_NOTE_FIRST)
.category(getName())
.subCategory("Merge Request Decoration")
.onQualifiers(Qualifiers.PROJECT)
.name("Submit summary note first")
.description("Submit summary discussion thread before issue comments.")
.type(PropertyType.BOOLEAN)
.defaultValue(String.valueOf(false))
.build());
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure this needs a configuration property. I'm happy that the plugin should remain opinionated and only implement configuration options that Sonarqube's core provides. I'd remove this and just go with the Summary note always being first if that's the direction you think the plugin should be going in

@@ -1 +1 @@
version=1.15.0-SNAPSHOT
version=1.14.2-SNAPSHOT
Copy link
Owner

Choose a reason for hiding this comment

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

Was there a reason for this version change? I presume you're rebased off a branch and picked up an old version number?

import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.Note;
import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.User;

import java.util.*;
Copy link
Owner

Choose a reason for hiding this comment

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

Please ensure your IDE is configured not to use star imports

@mc1arke
Copy link
Owner

mc1arke commented Dec 29, 2023

Closing as abandoned

@mc1arke mc1arke closed this Dec 29, 2023
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.

None yet

2 participants