Skip to content

Commit 7ec418c

Browse files
rtyleymzuehlke
authored andcommitted
Refactor pull request head: replace conditionals with polymorphism
This is another example of replacing conditionals with polymorphism on `ForgeType`. This was a theme in commit b42866b, merged in this PR back in August 2023: #3145 A benefit of this pattern is as new types of forge are implemented, the responsibilities for supporting that new `ForgeType` are all in one place, rather than spread across several methods elsewhere in the codebase. To confess, I think there's probably a further refactor that could be done, to introduce a better parameter object denoting a 'Pull Request Head' (that conceivably could better model the way that some `ForgeType`s don't even support forking) - but that can wait for another day, fixing #3300 is more urgent!
1 parent 2dd07ba commit 7ec418c

File tree

4 files changed

+30
-60
lines changed

4 files changed

+30
-60
lines changed

modules/core/src/main/scala/org/scalasteward/core/forge/ForgeType.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,13 @@ import cats.syntax.all._
2121
import org.http4s.Uri
2222
import org.http4s.syntax.literals._
2323
import org.scalasteward.core.application.Config.ForgeCfg
24+
import org.scalasteward.core.data.Repo
2425
import org.scalasteward.core.forge.ForgeType._
26+
import org.scalasteward.core.git.Branch
2527
import org.scalasteward.core.util.unexpectedString
2628

29+
import scala.annotation.nowarn
30+
2731
sealed trait ForgeType extends Product with Serializable {
2832
def publicWebHost: Option[String]
2933

@@ -41,6 +45,11 @@ sealed trait ForgeType extends Product with Serializable {
4145
def supportsForking: Boolean = true
4246
def supportsLabels: Boolean = true
4347

48+
/** Determines the `head` (GitHub) / `source_branch` (GitLab, Bitbucket) parameter for searching
49+
* for already existing pull requests or creating new pull requests.
50+
*/
51+
def pullRequestHeadFor(@nowarn fork: Repo, updateBranch: Branch): String = updateBranch.name
52+
4453
val asString: String = this match {
4554
case AzureRepos => "azure-repos"
4655
case Bitbucket => "bitbucket"
@@ -94,6 +103,8 @@ object ForgeType {
94103
val publicApiBaseUrl = uri"https://api.github.com"
95104
val diffs: DiffUriPattern = (from, to) => _ / "compare" / s"$from...$to"
96105
val files: FileUriPattern = fileName => _ / "blob" / "master" / fileName
106+
override def pullRequestHeadFor(fork: Repo, updateBranch: Branch): String =
107+
s"${fork.owner}:${updateBranch.name}"
97108
}
98109

99110
case object GitLab extends ForgeType {

modules/core/src/main/scala/org/scalasteward/core/forge/package.scala

Lines changed: 0 additions & 36 deletions
This file was deleted.

modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import org.scalasteward.core.git.{Branch, Commit, GitAlg}
3131
import org.scalasteward.core.repoconfig.PullRequestUpdateStrategy
3232
import org.scalasteward.core.util.logger.LoggerOps
3333
import org.scalasteward.core.util.{Nel, UrlChecker}
34-
import org.scalasteward.core.{forge, git, util}
34+
import org.scalasteward.core.{git, util}
3535
import org.typelevel.log4cats.Logger
3636

3737
final class NurtureAlg[F[_]](config: ForgeCfg)(implicit
@@ -92,7 +92,7 @@ final class NurtureAlg[F[_]](config: ForgeCfg)(implicit
9292
private def processUpdate(data: UpdateData): F[ProcessResult] =
9393
for {
9494
_ <- logger.info(s"Process update ${data.update.show}")
95-
head = forge.headFor(config.tpe, data.fork, data.updateBranch)
95+
head = config.tpe.pullRequestHeadFor(data.fork, data.updateBranch)
9696
pullRequests <- forgeApiAlg.listPullRequests(data.repo, head, data.baseBranch)
9797
result <- pullRequests.headOption match {
9898
case Some(pr) if pr.state.isClosed && data.update.isInstanceOf[Update.Single] =>
@@ -230,20 +230,18 @@ final class NurtureAlg[F[_]](config: ForgeCfg)(implicit
230230
.on(u => List(u.currentVersion.value), _.updates.map(_.currentVersion.value))
231231
.flatTraverse(gitAlg.findFilesContaining(data.repo, _))
232232
.map(_.distinct)
233-
branchName = forge.headFor(config.tpe, data.fork, data.updateBranch)
234233
allLabels = labelsFor(data.update, edits, filesWithOldVersion, artifactIdToVersionScheme)
235234
labels = filterLabels(allLabels, data.repoData.config.pullRequests.includeMatchedLabels)
236-
requestData = NewPullRequestData.from(
237-
data = data,
238-
branchName = branchName,
239-
edits = edits,
240-
artifactIdToUrl = artifactIdToUrl,
241-
artifactIdToUpdateInfoUrls = artifactIdToUpdateInfoUrls.toMap,
242-
filesWithOldVersion = filesWithOldVersion,
243-
addLabels = config.addLabels,
244-
labels = data.repoData.config.pullRequests.customLabels ++ labels
245-
)
246-
} yield requestData
235+
} yield NewPullRequestData.from(
236+
data = data,
237+
branchName = config.tpe.pullRequestHeadFor(data.fork, data.updateBranch),
238+
edits = edits,
239+
artifactIdToUrl = artifactIdToUrl,
240+
artifactIdToUpdateInfoUrls = artifactIdToUpdateInfoUrls.toMap,
241+
filesWithOldVersion = filesWithOldVersion,
242+
addLabels = config.addLabels,
243+
labels = data.repoData.config.pullRequests.customLabels ++ labels
244+
)
247245

248246
private def createPullRequest(data: UpdateData, edits: List[EditAttempt]): F[ProcessResult] =
249247
for {

modules/core/src/test/scala/org/scalasteward/core/forge/ForgePackageTest.scala renamed to modules/core/src/test/scala/org/scalasteward/core/forge/ForgeTypeTest.scala

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,24 @@ import org.scalasteward.core.data.{Repo, Update}
66
import org.scalasteward.core.forge.ForgeType.{GitHub, GitLab}
77
import org.scalasteward.core.git
88

9-
class ForgePackageTest extends FunSuite {
9+
class ForgeTypeTest extends FunSuite {
1010
private val repo = Repo("foo", "bar")
1111

1212
// Single updates
1313

1414
{
15-
1615
val update = ("ch.qos.logback".g % "logback-classic".a % "1.2.0" %> "1.2.3").single
1716
val updateBranch = git.branchFor(update, None)
1817

1918
test("headFor (single)") {
20-
assertEquals(headFor(GitHub, repo, updateBranch), s"foo:${updateBranch.name}")
21-
assertEquals(headFor(GitLab, repo, updateBranch), updateBranch.name)
19+
assertEquals(GitHub.pullRequestHeadFor(repo, updateBranch), s"foo:${updateBranch.name}")
20+
assertEquals(GitLab.pullRequestHeadFor(repo, updateBranch), updateBranch.name)
2221
}
23-
2422
}
2523

2624
// Grouped updates
2725

2826
{
29-
3027
val update = Update.Grouped(
3128
name = "my-group",
3229
title = None,
@@ -36,8 +33,8 @@ class ForgePackageTest extends FunSuite {
3633
val updateBranch = git.branchFor(update, None)
3734

3835
test("headFor (grouped)") {
39-
assertEquals(headFor(GitHub, repo, updateBranch), s"foo:update/my-group")
40-
assertEquals(headFor(GitLab, repo, updateBranch), updateBranch.name)
36+
assertEquals(GitHub.pullRequestHeadFor(repo, updateBranch), s"foo:update/my-group")
37+
assertEquals(GitLab.pullRequestHeadFor(repo, updateBranch), updateBranch.name)
4138
}
4239
}
4340

@@ -53,8 +50,8 @@ class ForgePackageTest extends FunSuite {
5350
val updateBranch = git.branchFor(update, None)
5451

5552
test("headFor (grouped) with $hash") {
56-
assertEquals(headFor(GitHub, repo, updateBranch), s"foo:update/my-group-1164623676")
57-
assertEquals(headFor(GitLab, repo, updateBranch), updateBranch.name)
53+
assertEquals(GitHub.pullRequestHeadFor(repo, updateBranch), s"foo:update/my-group-1164623676")
54+
assertEquals(GitLab.pullRequestHeadFor(repo, updateBranch), updateBranch.name)
5855
}
5956
}
6057

0 commit comments

Comments
 (0)