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

chore(bump): Update codacy-analysis-core to 7.9.4 #147

Merged
merged 15 commits into from
Nov 14, 2023

Conversation

mrfyda
Copy link
Contributor

@mrfyda mrfyda commented Nov 13, 2023

No description provided.

@mrfyda mrfyda marked this pull request as ready for review November 13, 2023 09:10
@mrfyda mrfyda enabled auto-merge (squash) November 13, 2023 09:12
Copy link
Member

@machadoit machadoit 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, especially if tests pass ;D But I think we will need to sync, for me to understand some details and where this new code is coming from

.vscode/settings.json Show resolved Hide resolved
@@ -25,10 +23,7 @@ nativeImageOptions ++= Seq("--enable-http",
"--no-fallback",
"--report-unsupported-elements-at-runtime")

// Scalafix

ThisBuild / scalafixDependencies += "com.github.liancheng" %% "organize-imports" % "0.6.0"
Copy link
Member

Choose a reason for hiding this comment

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

Was this removed temporarily and forgot to add back, or you want to propose to remove them?

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 included in the new version of scalafix

protected def createFullToolSpec(toolSpec: ToolSpec,
dockerToolDocumentation: DockerToolDocumentation): FullToolSpec = {
val patterns =
dockerToolDocumentation.toolSpecification
Copy link
Member

Choose a reason for hiding this comment

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

is the .toolSpecification the same as toolSpec? For me seems a bit strange that this method receives a toolSpec and a dockerToolDocumentation but then all the code is transformations only relying on dockerToolDocumentation.

Also assume that some if you replace some .map with .flatMap we may avoid some flattens / getOrElse? But we can take a look together

PluginPrefixHelper.prefixDescription(toolPrefix, patternDescriptions)
}

@deprecated(
Copy link
Member

Choose a reason for hiding this comment

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

Is this a copy paste form the codacy-plugins or something? Or what is the idea of this deprecated?

import java.nio.file.{Files, Path}
import scala.util.{Properties, Try}

class BinaryDockerHelper extends DockerHelper {
Copy link
Member

Choose a reason for hiding this comment

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

Similar to the other added files, are we adding because it got removed or something? Was this code on codacy-plugins before but now is not available?

@mrfyda
Copy link
Contributor Author

mrfyda commented Nov 13, 2023

Looks good, especially if tests pass ;D But I think we will need to sync, for me to understand some details and where this new code is coming from

sure 👍
the code was deleted from codacy-plugins (or moved) when codacy-tools was created
that's why we had a breaking change and the framework wasn't updated

Copy link
Member

@machadoit machadoit left a comment

Choose a reason for hiding this comment

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

LGTM!

@mrfyda mrfyda merged commit ed55b77 into master Nov 14, 2023
1 check passed
@delete-merged-branch delete-merged-branch bot deleted the update-codacy-analysis-core branch November 14, 2023 15:03
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.

2 participants