Skip to content

Add merging utilities #87

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

Merged
merged 7 commits into from
Feb 9, 2024
Merged

Add merging utilities #87

merged 7 commits into from
Feb 9, 2024

Conversation

matejdro
Copy link
Contributor

This fixes #82

Verified

This commit was signed with the committer’s verified signature.
bagder Daniel Stenberg
Copy link
Member

@BraisGabin BraisGabin left a comment

Choose a reason for hiding this comment

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

Thanks! Probably, in the future, we will need more tests but these is just a port from detekt so no need to add them in this PR.

matejdro and others added 3 commits January 29, 2024 13:32
Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
this should make it more reliable and ignore whitespace.
@BraisGabin BraisGabin enabled auto-merge (squash) January 29, 2024 20:32
@BraisGabin BraisGabin disabled auto-merge January 30, 2024 07:15
@BraisGabin
Copy link
Member

It seems that multiplatform doesn't like @JvmName.

Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
@BraisGabin BraisGabin requested a review from TWiStErRob February 1, 2024 21:06
Comment on lines +32 to +33
fun List<Run>.merge(other: List<Run>): List<Run> {
val runsByTool = (this + other).groupBy { it.tool.driver.fullName }
Copy link
Member

Choose a reason for hiding this comment

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

Thanks! Probably, in the future, we will need more tests but these is just a port from detekt so no need to add them in this PR.

@BraisGabin I think this is not just the port of the merger from Detekt, but also extra logic from detekt/detekt#6894, with an extra revision on top of it.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but that part I think it have tests. So, to me we can merge and prepare a release. Do you know how to do that?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Create a sarif merger
3 participants