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

Use RepoConfig#signoffCommits for migrations and hooks #3576

Merged
merged 1 commit into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,8 @@ final class EditAlg[F[_]](implicit
result <- logger.attemptWarn.log("Scalafix migration failed") {
buildToolDispatcher.runMigration(repo, config, migration)
}
maybeCommit <- gitAlg.commitAllIfDirty(
repo,
migration.commitMessage(result),
migration.signoffCommits
)
maybeCommit <-
gitAlg.commitAllIfDirty(repo, migration.commitMessage(result), config.signoffCommits)
} yield ScalafixEdit(migration, result, maybeCommit)

private def applyUpdateReplacements(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,21 @@ final class HookExecutor[F[_]](implicit
F: MonadThrow[F]
) {
def execPostUpdateHooks(data: RepoData, update: Update.Single): F[List[EditAttempt]] =
(HookExecutor.postUpdateHooks(
data.config.signoffCommits
) ++ data.config.postUpdateHooksOrDefault)
(HookExecutor.postUpdateHooks ++ data.config.postUpdateHooksOrDefault)
.filter { hook =>
hook.groupId.forall(update.groupId === _) &&
hook.artifactId.forall(aid => update.artifactIds.exists(_.name === aid.name)) &&
hook.enabledByCache(data.cache) &&
hook.enabledByConfig(data.config)
}
.distinctBy(_.command)
.traverse(execPostUpdateHook(data.repo, update, _))
.traverse(execPostUpdateHook(data.repo, update, _, data.config.signoffCommits))

private def execPostUpdateHook(
repo: Repo,
update: Update.Single,
hook: PostUpdateHook
hook: PostUpdateHook,
signoffCommits: Option[Boolean]
): F[EditAttempt] =
for {
_ <- logger.info(
Expand All @@ -77,10 +76,10 @@ final class HookExecutor[F[_]](implicit
commitMessage = hook
.commitMessage(update)
.appendParagraph(s"Executed command: ${hook.command.mkString_(" ")}")
maybeHookCommit <- gitAlg.commitAllIfDirty(repo, commitMessage, hook.signoffCommits)
maybeHookCommit <- gitAlg.commitAllIfDirty(repo, commitMessage, signoffCommits)
maybeBlameIgnoreCommit <-
maybeHookCommit.flatTraverse(
addToGitBlameIgnoreRevs(repo, repoDir, hook, _, commitMessage, hook.signoffCommits)
addToGitBlameIgnoreRevs(repo, repoDir, hook, _, commitMessage, signoffCommits)
)
} yield HookEdit(hook, result, maybeHookCommit.toList ++ maybeBlameIgnoreCommit.toList)

Expand Down Expand Up @@ -150,8 +149,7 @@ object HookExecutor {
private def sbtGithubWorkflowGenerateHook(
groupId: GroupId,
artifactId: ArtifactId,
enabledByCache: RepoCache => Boolean,
signoffCommits: Option[Boolean]
enabledByCache: RepoCache => Boolean
): PostUpdateHook =
PostUpdateHook(
groupId = Some(groupId),
Expand All @@ -161,11 +159,10 @@ object HookExecutor {
commitMessage = _ => CommitMsg("Regenerate GitHub Actions workflow"),
enabledByCache = enabledByCache,
enabledByConfig = _ => true,
addToGitBlameIgnoreRevs = false,
signoffCommits = signoffCommits
addToGitBlameIgnoreRevs = false
)

private def scalafmtHook(signoffCommits: Option[Boolean]) =
private val scalafmtHook =
PostUpdateHook(
groupId = Some(scalafmtGroupId),
artifactId = Some(scalafmtArtifactId),
Expand All @@ -174,14 +171,12 @@ object HookExecutor {
commitMessage = update => CommitMsg(s"Reformat with scalafmt ${update.nextVersion}"),
enabledByCache = _ => true,
enabledByConfig = _.scalafmtOrDefault.runAfterUpgradingOrDefault,
addToGitBlameIgnoreRevs = true,
signoffCommits = signoffCommits
addToGitBlameIgnoreRevs = true
)

private def sbtTypelevelHook(
groupId: GroupId,
artifactId: ArtifactId,
signoffCommits: Option[Boolean]
artifactId: ArtifactId
): PostUpdateHook =
PostUpdateHook(
groupId = Some(groupId),
Expand All @@ -191,22 +186,19 @@ object HookExecutor {
commitMessage = _ => CommitMsg("Run prePR with sbt-typelevel"),
enabledByCache = _ => true,
enabledByConfig = _ => true,
addToGitBlameIgnoreRevs = false,
signoffCommits = signoffCommits
addToGitBlameIgnoreRevs = false
)

private def githubWorkflowGenerateExists(cache: RepoCache): Boolean =
cache.dependsOn(sbtGitHubWorkflowGenerateModules ++ sbtTypelevelModules)

private def postUpdateHooks(signoffCommits: Option[Boolean]): List[PostUpdateHook] =
scalafmtHook(signoffCommits) ::
private val postUpdateHooks: List[PostUpdateHook] =
scalafmtHook ::
sbtGitHubWorkflowGenerateModules.map { case (gid, aid) =>
sbtGithubWorkflowGenerateHook(gid, aid, _ => true, signoffCommits)
sbtGithubWorkflowGenerateHook(gid, aid, _ => true)
} ++
conditionalSbtGitHubWorkflowGenerateModules.map { case (gid, aid) =>
sbtGithubWorkflowGenerateHook(gid, aid, githubWorkflowGenerateExists, signoffCommits)
sbtGithubWorkflowGenerateHook(gid, aid, githubWorkflowGenerateExists)
} ++
sbtTypelevelModules.map { case (gid, aid) =>
sbtTypelevelHook(gid, aid, signoffCommits)
}
sbtTypelevelModules.map { case (gid, aid) => sbtTypelevelHook(gid, aid) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ final case class PostUpdateHook(
commitMessage: Update.Single => CommitMsg,
enabledByCache: RepoCache => Boolean,
enabledByConfig: RepoConfig => Boolean,
addToGitBlameIgnoreRevs: Boolean,
signoffCommits: Option[Boolean]
addToGitBlameIgnoreRevs: Boolean
) {
def showCommand: String = command.mkString_("'", " ", "'")
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ final case class ScalafixMigration(
scalacOptions: Option[Nel[String]] = None,
authors: Option[Nel[Author]] = None,
target: Option[Target] = None,
executionOrder: Option[ExecutionOrder] = None,
signoffCommits: Option[Boolean]
executionOrder: Option[ExecutionOrder] = None
) {
def commitMessage(result: Either[Throwable, Unit]): CommitMsg = {
val verb = if (result.isRight) "Applied" else "Failed"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ final case class PostUpdateHookConfig(
artifactId: Option[String],
command: Nel[String],
commitMessage: String,
addToGitBlameIgnoreRevs: Option[Boolean] = None,
signoffCommits: Option[Boolean]
addToGitBlameIgnoreRevs: Option[Boolean] = None
) {
def toHook: PostUpdateHook =
PostUpdateHook(
Expand All @@ -40,8 +39,7 @@ final case class PostUpdateHookConfig(
commitMessage = CommitMsg.replaceVariables(commitMessage)(_, None),
enabledByCache = _ => true,
enabledByConfig = _ => true,
addToGitBlameIgnoreRevs = addToGitBlameIgnoreRevs.getOrElse(false),
signoffCommits = signoffCommits
addToGitBlameIgnoreRevs = addToGitBlameIgnoreRevs.getOrElse(false)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ class SbtAlgTest extends FunSuite {
GroupId("co.fs2"),
Nel.of("fs2-core"),
Version("1.0.0"),
Nel.of("github:functional-streams-for-scala/fs2/v1?sha=v1.0.5"),
signoffCommits = None
Nel.of("github:functional-streams-for-scala/fs2/v1?sha=v1.0.5")
)
val initialState = MockState.empty
.addFiles(
Expand Down Expand Up @@ -94,8 +93,7 @@ class SbtAlgTest extends FunSuite {
Nel.of("cats-core"),
Version("2.2.0"),
Nel.of("github:cb372/cats/Cats_v2_2_0?sha=235bd7c92e431ab1902db174cf4665b05e08f2f1"),
scalacOptions = Some(Nel.of("-P:semanticdb:synthetics:on")),
signoffCommits = None
scalacOptions = Some(Nel.of("-P:semanticdb:synthetics:on"))
)
val initialState = MockState.empty
.addFiles(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ class ScalaCliAlgTest extends CatsEffectSuite {
GroupId("co.fs2"),
Nel.of("fs2-core"),
Version("1.0.0"),
Nel.of("github:functional-streams-for-scala/fs2/v1?sha=v1.0.5"),
signoffCommits = None
Nel.of("github:functional-streams-for-scala/fs2/v1?sha=v1.0.5")
)
val obtained = scalaCliAlg.runMigration(buildRoot, migration).runS(MockState.empty)
val expected = MockState.empty.copy(trace =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ class HookExecutorTest extends CatsEffectSuite {
groupId = None,
artifactId = None,
command = Nel.of("sbt", "mySbtCommand"),
commitMessage = "Updated with a hook!",
signoffCommits = None
commitMessage = "Updated with a hook!"
)
).some
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ class ScalafixMigrationsFinderTest extends FunSuite {
Some(
"https://github.com/typelevel/cats/blob/v2.2.0/scalafix/README.md#migration-to-cats-v220"
),
Some(Nel.of("-P:semanticdb:synthetics:on")),
signoffCommits = None
Some(Nel.of("-P:semanticdb:synthetics:on"))
)
),
List()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ class ScalafixMigrationsLoaderTest extends FunSuite {
Some("https://scalacenter.github.io/scalafix/"),
authors = Some(Nel.of(Author("Jane Doe", "jane@example.com"))),
target = Some(Sources),
executionOrder = Some(PreUpdate),
signoffCommits = None
executionOrder = Some(PreUpdate)
)

test("loadAll: without extra file, without defaults") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ class NewPullRequestDataTest extends FunSuite {
groupId = "com.spotify".g,
artifactIds = Nel.one("scio-core"),
newVersion = Version("0.7.0"),
rewriteRules = Nel.of("I am a rewrite rule"),
signoffCommits = None
rewriteRules = Nel.of("I am a rewrite rule")
),
result = Right(()),
maybeCommit = Some(Commit(dummySha1))
Expand Down Expand Up @@ -464,8 +463,7 @@ class NewPullRequestDataTest extends FunSuite {
"com.spotify".g,
Nel.one("scio-core"),
Version("0.7.0"),
Nel.of("I am a rewrite rule"),
signoffCommits = None
Nel.of("I am a rewrite rule")
),
Right(()),
Some(Commit(dummySha1))
Expand Down Expand Up @@ -495,8 +493,7 @@ class NewPullRequestDataTest extends FunSuite {
Nel.one("scio-core"),
Version("0.7.0"),
Nel.of("I am a rewrite rule", "I am a 2nd rewrite rule"),
Some("https://scalacenter.github.io/scalafix/"),
signoffCommits = None
Some("https://scalacenter.github.io/scalafix/")
),
Right(()),
Some(Commit(dummySha1))
Expand Down Expand Up @@ -528,8 +525,7 @@ class NewPullRequestDataTest extends FunSuite {
Nel.one("scio-core"),
Version("0.7.0"),
Nel.of("I am a rewrite rule", "I am a 2nd rewrite rule"),
Some("https://scalacenter.github.io/scalafix/"),
signoffCommits = None
Some("https://scalacenter.github.io/scalafix/")
),
Right(()),
Some(Commit(dummySha1))
Expand All @@ -539,8 +535,7 @@ class NewPullRequestDataTest extends FunSuite {
"org.typeleve".g,
Nel.of("cats-effect", "cats-effect-laws"),
Version("3.0.0"),
Nel.of("I am a rule without an effect"),
signoffCommits = None
Nel.of("I am a rule without an effect")
),
Right(()),
None
Expand Down Expand Up @@ -623,8 +618,7 @@ class NewPullRequestDataTest extends FunSuite {
Nel.one("scio-core"),
Version("0.7.0"),
Nel.of("I am a rewrite rule", "I am a 2nd rewrite rule"),
Some("https://scalacenter.github.io/scalafix/"),
signoffCommits = None
Some("https://scalacenter.github.io/scalafix/")
),
Right(()),
Some(Commit(dummySha1))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,7 @@ class RepoConfigAlgTest extends FunSuite {
groupId = None,
artifactId = None,
command = Nel.of("sbt", "mySbtCommand"),
commitMessage = "Updated with a hook!",
signoffCommits = None
commitMessage = "Updated with a hook!"
)
).some
)
Expand All @@ -418,8 +417,7 @@ class RepoConfigAlgTest extends FunSuite {
groupId = Some("eu.timepit".g),
artifactId = Some("refined.1"),
command = Nel.of("sbt", "mySbtCommand"),
commitMessage = "Updated with a hook!",
signoffCommits = None
commitMessage = "Updated with a hook!"
)
).some
)
Expand Down