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

Add repo-specific signoff option #3432

Merged
merged 1 commit into from
Oct 13, 2024
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
4 changes: 4 additions & 0 deletions docs/repo-specific-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ dependencyOverrides = [
# to add assignees or request reviews. Consequently, it won't work for public @scala-steward instance on GitHub.
assignees = [ "username1", "username2" ]
reviewers = [ "username1", "username2" ]

# If true, Scala Steward will sign off all commits (e.g. `git --signoff`).
# Default: false
signoffCommits = true
```

The version information given in the patterns above can be in two formats:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ final class EditAlg[F[_]](implicit
result <- logger.attemptWarn.log("Scalafix migration failed") {
buildToolDispatcher.runMigration(repo, config, migration)
}
maybeCommit <- gitAlg.commitAllIfDirty(repo, migration.commitMessage(result))
maybeCommit <- gitAlg.commitAllIfDirty(
repo,
migration.commitMessage(result),
migration.signoffCommits
)
} yield ScalafixEdit(migration, result, maybeCommit)

private def applyUpdateReplacements(
Expand All @@ -111,7 +115,7 @@ final class EditAlg[F[_]](implicit
_ <- reformatChangedFiles(data)
msgTemplate = data.config.commits.messageOrDefault
commitMsg = CommitMsg.replaceVariables(msgTemplate)(update, data.repo.branch)
maybeCommit <- gitAlg.commitAllIfDirty(data.repo, commitMsg)
maybeCommit <- gitAlg.commitAllIfDirty(data.repo, commitMsg, data.config.signoffCommits)
} yield maybeCommit.map(UpdateEdit(update, _))

private def reformatChangedFiles(data: RepoData): F[Unit] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ final class HookExecutor[F[_]](implicit
F: MonadThrow[F]
) {
def execPostUpdateHooks(data: RepoData, update: Update.Single): F[List[EditAttempt]] =
(HookExecutor.postUpdateHooks ++ data.config.postUpdateHooksOrDefault)
(HookExecutor.postUpdateHooks(
data.config.signoffCommits
) ++ data.config.postUpdateHooksOrDefault)
.filter { hook =>
hook.groupId.forall(update.groupId === _) &&
hook.artifactId.forall(aid => update.artifactIds.exists(_.name === aid.name)) &&
Expand Down Expand Up @@ -75,17 +77,20 @@ final class HookExecutor[F[_]](implicit
commitMessage = hook
.commitMessage(update)
.appendParagraph(s"Executed command: ${hook.command.mkString_(" ")}")
maybeHookCommit <- gitAlg.commitAllIfDirty(repo, commitMessage)
maybeHookCommit <- gitAlg.commitAllIfDirty(repo, commitMessage, hook.signoffCommits)
maybeBlameIgnoreCommit <-
maybeHookCommit.flatTraverse(addToGitBlameIgnoreRevs(repo, repoDir, hook, _, commitMessage))
maybeHookCommit.flatTraverse(
addToGitBlameIgnoreRevs(repo, repoDir, hook, _, commitMessage, hook.signoffCommits)
)
} yield HookEdit(hook, result, maybeHookCommit.toList ++ maybeBlameIgnoreCommit.toList)

private def addToGitBlameIgnoreRevs(
repo: Repo,
repoDir: File,
hook: PostUpdateHook,
commit: Commit,
commitMsg: CommitMsg
commitMsg: CommitMsg,
signoffCommits: Option[Boolean]
): F[Option[Commit]] =
if (hook.addToGitBlameIgnoreRevs) {
for {
Expand All @@ -100,7 +105,7 @@ final class HookExecutor[F[_]](implicit
addAndCommit = gitAlg.add(repo, pathAsString).flatMap { _ =>
val blameIgnoreCommitMsg =
CommitMsg(s"Add '${commitMsg.title}' to $gitBlameIgnoreRevsName")
gitAlg.commitAllIfDirty(repo, blameIgnoreCommitMsg)
gitAlg.commitAllIfDirty(repo, blameIgnoreCommitMsg, signoffCommits)
}
maybeBlameIgnoreCommit <- gitAlg
.checkIgnore(repo, pathAsString)
Expand Down Expand Up @@ -145,7 +150,8 @@ object HookExecutor {
private def sbtGithubWorkflowGenerateHook(
groupId: GroupId,
artifactId: ArtifactId,
enabledByCache: RepoCache => Boolean
enabledByCache: RepoCache => Boolean,
signoffCommits: Option[Boolean]
): PostUpdateHook =
PostUpdateHook(
groupId = Some(groupId),
Expand All @@ -155,10 +161,11 @@ object HookExecutor {
commitMessage = _ => CommitMsg("Regenerate GitHub Actions workflow"),
enabledByCache = enabledByCache,
enabledByConfig = _ => true,
addToGitBlameIgnoreRevs = false
addToGitBlameIgnoreRevs = false,
signoffCommits = signoffCommits
)

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

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

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

private val postUpdateHooks: List[PostUpdateHook] =
scalafmtHook ::
private def postUpdateHooks(signoffCommits: Option[Boolean]): List[PostUpdateHook] =
scalafmtHook(signoffCommits) ::
sbtGitHubWorkflowGenerateModules.map { case (gid, aid) =>
sbtGithubWorkflowGenerateHook(gid, aid, _ => true)
sbtGithubWorkflowGenerateHook(gid, aid, _ => true, signoffCommits)
} ++
conditionalSbtGitHubWorkflowGenerateModules.map { case (gid, aid) =>
sbtGithubWorkflowGenerateHook(gid, aid, githubWorkflowGenerateExists)
sbtGithubWorkflowGenerateHook(gid, aid, githubWorkflowGenerateExists, signoffCommits)
} ++
sbtTypelevelModules.map { case (gid, aid) =>
sbtTypelevelHook(gid, aid)
sbtTypelevelHook(gid, aid, signoffCommits)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ final case class PostUpdateHook(
commitMessage: Update.Single => CommitMsg,
enabledByCache: RepoCache => Boolean,
enabledByConfig: RepoConfig => Boolean,
addToGitBlameIgnoreRevs: Boolean
addToGitBlameIgnoreRevs: Boolean,
signoffCommits: Option[Boolean]
) {
def showCommand: String = command.mkString_("'", " ", "'")
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ final case class ScalafixMigration(
scalacOptions: Option[Nel[String]] = None,
authors: Option[Nel[Author]] = None,
target: Option[Target] = None,
executionOrder: Option[ExecutionOrder] = None
executionOrder: Option[ExecutionOrder] = None,
signoffCommits: Option[Boolean]
) {
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 @@ -63,10 +63,16 @@ final class FileGitAlg[F[_]](config: GitCfg)(implicit
override def cloneExists(repo: File): F[Boolean] =
fileAlg.isDirectory(repo / ".git")

override def commitAll(repo: File, message: CommitMsg): F[Commit] = {
override def commitAll(
repo: File,
message: CommitMsg,
signoffCommits: Option[Boolean]
): F[Commit] = {
val messages = message.paragraphs.foldMap(m => List("-m", m))
val trailers = message.trailers.foldMap { case (k, v) => List("--trailer", s"$k=$v") }
git_("commit" :: "--all" :: sign :: signoff :: messages ++ trailers: _*)(repo) >>
git_("commit" :: "--all" :: sign :: signoff(signoffCommits) :: messages ++ trailers: _*)(
repo
) >>
latestSha1(repo, Branch.head).map(Commit.apply)
}

Expand Down Expand Up @@ -171,8 +177,8 @@ final class FileGitAlg[F[_]](config: GitCfg)(implicit
private val sign: String =
if (config.signCommits) "--gpg-sign" else "--no-gpg-sign"

private val signoff: String =
if (config.signoff) "--signoff" else "--no-signoff"
private def signoff(signoffCommits: Option[Boolean]): String =
if (signoffCommits.getOrElse(config.signoff)) "--signoff" else "--no-signoff"
}

object FileGitAlg {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ trait GenGitAlg[F[_], Repo] {

def cloneExists(repo: Repo): F[Boolean]

def commitAll(repo: Repo, message: CommitMsg): F[Commit]
def commitAll(repo: Repo, message: CommitMsg, signoffCommits: Option[Boolean]): F[Commit]

def containsChanges(repo: Repo): F[Boolean]

Expand Down Expand Up @@ -78,10 +78,13 @@ trait GenGitAlg[F[_], Repo] {

def version: F[String]

final def commitAllIfDirty(repo: Repo, message: CommitMsg)(implicit
F: Monad[F]
final def commitAllIfDirty(repo: Repo, message: CommitMsg, signoffCommits: Option[Boolean])(
implicit F: Monad[F]
): F[Option[Commit]] =
containsChanges(repo).ifM(commitAll(repo, message).map(Some.apply), F.pure(None))
containsChanges(repo).ifM(
commitAll(repo, message, signoffCommits).map(Some.apply),
F.pure(None)
)

final def returnToCurrentBranch[A, E](repo: Repo)(fa: F[A])(implicit F: MonadCancel[F, E]): F[A] =
F.bracket(currentBranch(repo))(_ => fa)(checkoutBranch(repo, _))
Expand Down Expand Up @@ -113,8 +116,12 @@ trait GenGitAlg[F[_], Repo] {
override def cloneExists(repo: A): F[Boolean] =
f(repo).flatMap(self.cloneExists)

override def commitAll(repo: A, message: CommitMsg): F[Commit] =
f(repo).flatMap(self.commitAll(_, message))
override def commitAll(
repo: A,
message: CommitMsg,
signoffCommits: Option[Boolean]
): F[Commit] =
f(repo).flatMap(self.commitAll(_, message, signoffCommits))

override def containsChanges(repo: A): F[Boolean] =
f(repo).flatMap(self.containsChanges)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ final case class PostUpdateHookConfig(
artifactId: Option[String],
command: Nel[String],
commitMessage: String,
addToGitBlameIgnoreRevs: Option[Boolean] = None
addToGitBlameIgnoreRevs: Option[Boolean] = None,
signoffCommits: Option[Boolean]
) {
def toHook: PostUpdateHook =
PostUpdateHook(
Expand All @@ -40,7 +41,8 @@ final case class PostUpdateHookConfig(
commitMessage = CommitMsg.replaceVariables(commitMessage)(_, None),
enabledByCache = _ => true,
enabledByConfig = _ => true,
addToGitBlameIgnoreRevs = addToGitBlameIgnoreRevs.getOrElse(false)
addToGitBlameIgnoreRevs = addToGitBlameIgnoreRevs.getOrElse(false),
signoffCommits = signoffCommits
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ final case class RepoConfig(
buildRoots: Option[List[BuildRootConfig]] = None,
assignees: List[String] = List.empty,
reviewers: List[String] = List.empty,
dependencyOverrides: List[GroupRepoConfig] = List.empty
dependencyOverrides: List[GroupRepoConfig] = List.empty,
signoffCommits: Option[Boolean] = None
) {
def buildRootsOrDefault(repo: Repo): List[BuildRoot] =
buildRoots
Expand Down Expand Up @@ -88,7 +89,8 @@ object RepoConfig {
buildRoots = x.buildRoots |+| y.buildRoots,
assignees = x.assignees |+| y.assignees,
reviewers = x.reviewers |+| y.reviewers,
dependencyOverrides = x.dependencyOverrides |+| y.dependencyOverrides
dependencyOverrides = x.dependencyOverrides |+| y.dependencyOverrides,
signoffCommits = x.signoffCommits.orElse(y.signoffCommits)
)
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ 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")
Nel.of("github:functional-streams-for-scala/fs2/v1?sha=v1.0.5"),
signoffCommits = None
)
val initialState = MockState.empty
.addFiles(
Expand Down Expand Up @@ -93,7 +94,8 @@ 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"))
scalacOptions = Some(Nel.of("-P:semanticdb:synthetics:on")),
signoffCommits = None
)
val initialState = MockState.empty
.addFiles(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ 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")
Nel.of("github:functional-streams-for-scala/fs2/v1?sha=v1.0.5"),
signoffCommits = None
)
val obtained = scalaCliAlg.runMigration(buildRoot, migration).runS(MockState.empty)
assertIO(obtained.map(_.trace.collect { case Log(_) => () }.size), 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ class HookExecutorTest extends CatsEffectSuite {
groupId = None,
artifactId = None,
command = Nel.of("sbt", "mySbtCommand"),
commitMessage = "Updated with a hook!"
commitMessage = "Updated with a hook!",
signoffCommits = None
)
).some
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ 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"))
Some(Nel.of("-P:semanticdb:synthetics:on")),
signoffCommits = None
)
),
List()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ 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)
executionOrder = Some(PreUpdate),
signoffCommits = None
)

test("loadAll: without extra file, without defaults") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ 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")
rewriteRules = Nel.of("I am a rewrite rule"),
signoffCommits = None
),
result = Right(()),
maybeCommit = Some(Commit(dummySha1))
Expand Down Expand Up @@ -317,7 +318,8 @@ class NewPullRequestDataTest extends FunSuite {
"com.spotify".g,
Nel.one("scio-core"),
Version("0.7.0"),
Nel.of("I am a rewrite rule")
Nel.of("I am a rewrite rule"),
signoffCommits = None
),
Right(()),
Some(Commit(dummySha1))
Expand Down Expand Up @@ -347,7 +349,8 @@ 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/")
Some("https://scalacenter.github.io/scalafix/"),
signoffCommits = None
),
Right(()),
Some(Commit(dummySha1))
Expand Down Expand Up @@ -379,7 +382,8 @@ 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/")
Some("https://scalacenter.github.io/scalafix/"),
signoffCommits = None
),
Right(()),
Some(Commit(dummySha1))
Expand All @@ -389,7 +393,8 @@ 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")
Nel.of("I am a rule without an effect"),
signoffCommits = None
),
Right(()),
None
Expand Down Expand Up @@ -472,7 +477,8 @@ 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/")
Some("https://scalacenter.github.io/scalafix/"),
signoffCommits = None
),
Right(()),
Some(Commit(dummySha1))
Expand Down
Loading
Loading