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

Proposal: Improve branch selection #2281

Merged
merged 29 commits into from
Oct 18, 2021
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6e4d716
Add optional `Branch` param to `Repo` class
alejandrohdezma Oct 13, 2021
d7f5338
Populate branch for repos added as `- org/repo:branch`
alejandrohdezma Oct 13, 2021
0449cbc
Use branch from `Repo` class instead of configuration
alejandrohdezma Oct 13, 2021
ab00c61
Remove `--default-branch` CLI param
alejandrohdezma Oct 13, 2021
b0a8e0a
Run `sbt fmt`
alejandrohdezma Oct 13, 2021
ee9ab63
Update `Repo#show` to include branch information if present
alejandrohdezma Oct 13, 2021
88015c9
Ensure PR title includes the branch if it is different than the defau…
alejandrohdezma Oct 13, 2021
2f21827
Propagate branch to missing places
alejandrohdezma Oct 16, 2021
02266c3
Merge branch 'master' into move-default-branch-selection-to-repos-file
alejandrohdezma Oct 16, 2021
0ac47d4
Run `sbt fmt`
alejandrohdezma Oct 16, 2021
c4bf4e5
Remove unused import
alejandrohdezma Oct 16, 2021
cc52559
Merge remote-tracking branch 'upstream/master' into move-default-bran…
alejandrohdezma Oct 16, 2021
38fec63
Add repo with branch to repos file
fthomas Oct 16, 2021
9a3830b
Do not check out branch if forking is enabled
fthomas Oct 16, 2021
899a019
Revert changes to the repoKeyEncoder
fthomas Oct 16, 2021
a8e10ef
Ensure correct branch is checked-out when syncing a fork
alejandrohdezma Oct 16, 2021
e848496
Fix GitHub branch listing for forks
alejandrohdezma Oct 17, 2021
23c7675
`vcsRepoAlg.cloenAndSync` has already checked-out the correct branch …
alejandrohdezma Oct 17, 2021
6ae80b7
Merge branch 'master' into move-default-branch-selection-to-repos-file
fthomas Oct 17, 2021
67ed0cf
Update documentation
alejandrohdezma Oct 17, 2021
462e97f
parent.default_branch is already the same as repo.branch
fthomas Oct 17, 2021
f1150d9
Fix format of the example repos.md file
fthomas Oct 17, 2021
0c82713
Minimize diff
fthomas Oct 17, 2021
dfe8b4c
Shorter show and toPath implementations
fthomas Oct 17, 2021
104307a
Add `.vscode` folder to `.gitignore`
alejandrohdezma Oct 17, 2021
00e942f
Propagate `UpdateDate#baseBranch` to `commitMsgFor`
alejandrohdezma Oct 17, 2021
7386dbc
Revert "Propagate `UpdateDate#baseBranch` to `commitMsgFor`"
fthomas Oct 17, 2021
c6d7605
Add val for commit message
fthomas Oct 17, 2021
2cc0c9b
Fix Repo.{show, toPath}
fthomas Oct 18, 2021
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
16 changes: 15 additions & 1 deletion docs/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,27 @@ Updates for `sbt` and `scalafmt` can be controlled by using the following `group
{ groupId = "org.scalameta", artifactId = "scalafmt-core" }
```

## Can Scala Steward update multiple branches in a repository?

Yes! You can update multiple branches of a repository by adding it several times to the "repos.md" file
and a suffix with a different branch on each occurence (`owner/repo:branch`). For example:

```md
// repos.md
- owner/repo
- owner/repo:0.1.x
- owner/repo:0.2.x
```

This configuration will update the default branch, as well as the branches `0.1.x` and `0.2.x` in the repo `owner/repo`.

## Can Scala Steward update dependencies in giter8 templates ?

Scala Steward can update versions in giter8 templates if the dependencies of the template
are also added as dependencies of the template build.
An example is [library.g8](https://github.com/ChristopherDavenport/library.g8) ([example PR](https://github.com/ChristopherDavenport/library.g8/pull/100/files))

## Why do Scala Steward updates provide no URLs in PRs
## Why do Scala Steward updates provide no URLs in PRs?

Scala Steward updates can only provide links to release notes, diffs and changelogs if
the `ivy.xml` file contain the `homepage` attribute or the `pom.xml` contains either
Expand Down
1 change: 0 additions & 1 deletion docs/help.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,4 @@ All command line arguments for the `scala-steward` application.
--github-app-key-file FILE Github application key file
--github-app-id ID Github application id
--refresh-backoff-period DURATION Period of time a failed build won't be triggered again, default: "7 days"
--default-branch BRANCH A fixed branch name to use as default instead of the repository's default branch
```
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ object Cli {
githubAppId: Option[Long] = None,
urlCheckerTestUrl: Option[Uri] = None,
defaultMavenRepo: Option[String] = None,
refreshBackoffPeriod: FiniteDuration = 7.days,
defaultBranch: Option[String] = None
refreshBackoffPeriod: FiniteDuration = 7.days
)

final case class EnvVar(name: String, value: String)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import org.http4s.syntax.literals._
import org.scalasteward.core.application.Cli.EnvVar
import org.scalasteward.core.application.Config._
import org.scalasteward.core.data.Resolver
import org.scalasteward.core.git.{Author, Branch}
import org.scalasteward.core.git.Author
import org.scalasteward.core.io.{ProcessAlg, WorkspaceAlg}
import org.scalasteward.core.util
import org.scalasteward.core.util.Nel
Expand Down Expand Up @@ -74,8 +74,7 @@ final case class Config(
githubApp: Option[GitHubApp],
urlCheckerTestUrl: Uri,
defaultResolver: Resolver,
refreshBackoffPeriod: FiniteDuration,
defaultBranch: Option[Branch]
refreshBackoffPeriod: FiniteDuration
) {
def vcsUser[F[_]](implicit
processAlg: ProcessAlg[F],
Expand Down Expand Up @@ -184,7 +183,6 @@ object Config {
defaultResolver = args.defaultMavenRepo
.map(url => Resolver.MavenRepository("default", url, None))
.getOrElse(Resolver.mavenCentral),
refreshBackoffPeriod = args.refreshBackoffPeriod,
defaultBranch = args.defaultBranch.map(Branch(_))
refreshBackoffPeriod = args.refreshBackoffPeriod
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import org.scalasteward.core.vcs.data.Repo
import org.scalasteward.core.vcs.github.{GitHubApp, GitHubAppApiAlg, GitHubAuthAlg}
import org.typelevel.log4cats.Logger
import scala.concurrent.duration._
import org.scalasteward.core.git.Branch

final class StewardAlg[F[_]](config: Config)(implicit
dateTimeAlg: DateTimeAlg[F],
Expand All @@ -53,9 +54,12 @@ final class StewardAlg[F[_]](config: Config)(implicit
Stream.evals[F, List, Repo] {
fileAlg.readFile(reposFile).map { maybeContent =>
val regex = """-\s+(.+)/([^/]+)""".r
val regexWithBranch = """-\s+(.+)/([^/]+):([^/]+)""".r
val content = maybeContent.getOrElse("")
content.linesIterator.collect { case regex(owner, repo) =>
Repo(owner.trim, repo.trim)
content.linesIterator.collect {
case regexWithBranch(owner, repo, branch) =>
Repo(owner.trim, repo.trim, Some(Branch(branch.trim)))
case regex(owner, repo) => Repo(owner.trim, repo.trim)
}.toList
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ final class EditAlg[F[_]](implicit
runScalafixMigrations(repo, data.config, migrations) <*
bumpVersion(update, files)
updateEdit <- gitAlg
.commitAllIfDirty(repo, git.commitMsgFor(update, data.config.commits))
.commitAllIfDirty(
repo,
git.commitMsgFor(update, data.config.commits, data.repo.branch)
)
.map(_.map(commit => UpdateEdit(update, commit)))
hooksEdits <- hookExecutor.execPostUpdateHooks(data, update)
} yield scalafixEdits ++ updateEdit ++ hooksEdits
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.scalasteward.core

import cats.syntax.all._
import org.scalasteward.core.data.Update
import org.scalasteward.core.repoconfig.CommitsConfig
import org.scalasteward.core.update.show
Expand All @@ -24,16 +25,22 @@ import org.scalasteward.core.vcs.data.Repo
package object git {
type GitAlg[F[_]] = GenGitAlg[F, Repo]

def branchFor(update: Update): Branch =
Branch(s"update/${update.name}-${update.nextVersion}")
def branchFor(update: Update, baseBranch: Option[Branch]): Branch =
baseBranch
.map(branch => Branch(s"update/${branch.name}/${update.name}-${update.nextVersion}"))
.getOrElse(Branch(s"update/${update.name}-${update.nextVersion}"))

def commitMsgFor(update: Update, commitsConfig: CommitsConfig): String = {
def commitMsgFor(update: Update, commitsConfig: CommitsConfig, branch: Option[Branch]): String = {
fthomas marked this conversation as resolved.
Show resolved Hide resolved
val artifact = show.oneLiner(update)
val defaultMessage = s"Update $artifact to ${update.nextVersion}"
val defaultMessage = branch match {
case Some(value) => s"Update $artifact to ${update.nextVersion} in ${value.name}"
case None => s"Update $artifact to ${update.nextVersion}"
}
commitsConfig.messageOrDefault
.replace("${default}", defaultMessage)
.replace("${artifactName}", artifact)
.replace("${currentVersion}", update.currentVersion)
.replace("${nextVersion}", update.nextVersion)
.replace("${branchName}", branch.map(_.name).orEmpty)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ final class NurtureAlg[F[_]](config: VCSCfg)(implicit
_ <- NurtureAlg.processUpdates(
grouped,
update => {
val updateData =
UpdateData(data, fork, update, baseBranch, baseSha1, git.branchFor(update))
val updateBranch = git.branchFor(update, data.repo.branch)
val updateData = UpdateData(data, fork, update, baseBranch, baseSha1, updateBranch)
processUpdate(updateData)
},
data.config.updates.limit
Expand Down Expand Up @@ -130,7 +130,7 @@ final class NurtureAlg[F[_]](config: VCSCfg)(implicit
_ <- pullRequestRepository.changeState(data.repo, oldUrl, PullRequestState.Closed)
comment = s"Superseded by ${vcsApiAlg.referencePullRequest(newNumber)}."
_ <- vcsApiAlg.commentPullRequest(data.repo, oldNumber, comment)
oldBranch = git.branchFor(oldUpdate)
oldBranch = git.branchFor(oldUpdate, data.repo.branch)
oldRemoteBranch = oldBranch.withPrefix("origin/")
oldBranchExists <- gitAlg.branchExists(data.repo, oldRemoteBranch)
authors <-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,7 @@ final class RepoCacheAlg[F[_]](config: Config)(implicit
logger.info(s"Check cache of ${repo.show}") >>
refreshErrorAlg.skipIfFailedRecently(repo) {
(
vcsApiAlg.createForkOrGetRepoWithDefaultBranch(
repo,
config.vcsCfg.doNotFork,
config.defaultBranch
),
vcsApiAlg.createForkOrGetRepoWithBranch(repo, config.vcsCfg.doNotFork),
repoCacheRepository.findCache(repo)
).parTupled.flatMap { case ((repoOut, branchOut), maybeCache) =>
val latestSha1 = branchOut.commit.sha
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,27 +42,15 @@ trait VCSApiAlg[F[_]] {
final def createForkOrGetRepo(repo: Repo, doNotFork: Boolean): F[RepoOut] =
if (doNotFork) getRepo(repo) else createFork(repo)

final def createForkOrGetRepoWithDefaultBranch(
repo: Repo,
doNotFork: Boolean,
defaultBranch: Option[Branch]
)(implicit
final def createForkOrGetRepoWithBranch(repo: Repo, doNotFork: Boolean)(implicit
F: MonadThrow[F]
): F[(RepoOut, BranchOut)] =
for {
forkOrRepo <- createForkOrGetRepo(repo, doNotFork)
forkOrRepoWithDefaultBranch = applyDefaultBranch(forkOrRepo, defaultBranch)
forkOrRepoWithDefaultBranch = repo.branch.fold(forkOrRepo)(forkOrRepo.withBranch)
defaultBranch <- getDefaultBranchOfParentOrRepo(forkOrRepoWithDefaultBranch, doNotFork)
} yield (forkOrRepoWithDefaultBranch, defaultBranch)

final def applyDefaultBranch(repoOut: RepoOut, defaultBranch: Option[Branch]): RepoOut =
defaultBranch.fold(repoOut) { branch =>
repoOut.copy(
default_branch = branch,
parent = repoOut.parent.map(_.copy(default_branch = branch))
)
}

final def getDefaultBranchOfParentOrRepo(repoOut: RepoOut, doNotFork: Boolean)(implicit
F: MonadThrow[F]
): F[BranchOut] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,12 @@ final class VCSRepoAlg[F[_]](config: Config)(implicit
F: MonadThrow[F]
) {
def cloneAndSync(repo: Repo, repoOut: RepoOut): F[Unit] =
clone(repo, repoOut) >> maybeSyncFork(repo, repoOut) >> initSubmodules(repo)
clone(repo, repoOut) >> maybeCheckoutBranchOrSyncFork(repo, repoOut) >> initSubmodules(repo)

private def clone(repo: Repo, repoOut: RepoOut): F[Unit] =
logger.info(s"Clone ${repoOut.repo.show}") >>
gitAlg.clone(repo, withLogin(repoOut.clone_url)).adaptErr(adaptCloneError) >>
gitAlg.setAuthor(repo, config.gitCfg.gitAuthor) >>
config.defaultBranch.fold(F.unit)(gitAlg.checkoutBranch(repo, _))
gitAlg.setAuthor(repo, config.gitCfg.gitAuthor)

private val adaptCloneError: PartialFunction[Throwable, Throwable] = {
case throwable if config.vcsCfg.tpe === GitHub && !config.vcsCfg.doNotFork =>
Expand All @@ -52,8 +51,9 @@ final class VCSRepoAlg[F[_]](config: Config)(implicit
new Throwable(message, throwable)
}

private def maybeSyncFork(repo: Repo, repoOut: RepoOut): F[Unit] =
if (config.vcsCfg.doNotFork) F.unit else syncFork(repo, repoOut)
private def maybeCheckoutBranchOrSyncFork(repo: Repo, repoOut: RepoOut): F[Unit] =
if (config.vcsCfg.doNotFork) repo.branch.fold(F.unit)(gitAlg.checkoutBranch(repo, _))
else syncFork(repo, repoOut)

private def syncFork(repo: Repo, repoOut: RepoOut): F[Unit] =
repoOut.parentOrRaise[F].flatMap { parent =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class BitbucketApiAlg[F[_]](config: VCSCfg, modify: Repo => Request[F] => F[Requ
val payload = CreatePullRequestRequest(
data.title,
Branch(data.head),
Repo(sourceBranchOwner, repo.repo),
Repo(sourceBranchOwner, repo.repo, repo.branch),
data.base,
data.body
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ private[bitbucket] object RepositoryResponse {
implicit private val repoDecoder: Decoder[Repo] = Decoder.instance { c =>
c.as[String].map(_.split('/')).flatMap { parts =>
parts match {
case Array(owner, name) => Repo(owner, name).asRight
case _ => DecodingFailure("Repo", c.history).asLeft
case Array(owner, name) => Repo(owner, name).asRight
case Array(owner, name, branch) => Repo(owner, name, Some(Branch(branch))).asRight
case _ => DecodingFailure("Repo", c.history).asLeft
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ object NewPullRequestData {
filesWithOldVersion: List[String] = List.empty
): NewPullRequestData =
NewPullRequestData(
title = git.commitMsgFor(data.update, data.repoConfig.commits),
title = git.commitMsgFor(data.update, data.repoConfig.commits, data.repoData.repo.branch),
body = bodyFor(data.update, edits, artifactIdToUrl, releaseRelatedUrls, filesWithOldVersion),
head = branchName,
base = data.baseBranch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,22 @@ package org.scalasteward.core.vcs.data

import cats.Eq
import io.circe.KeyEncoder
import org.scalasteward.core.git.Branch

final case class Repo(
owner: String,
repo: String
repo: String,
branch: Option[Branch] = None
) {
def show: String = s"$owner/$repo"
def show: String = branch match {
case Some(value) => s"$owner/$repo:${value.name}"
case None => s"$owner/$repo"
}

def toPath: String = s"$owner/$repo"
def toPath: String = branch match {
case Some(value) => s"$owner/$repo/${value.name}"
case None => s"$owner/$repo"
}
}

object Repo {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ final case class RepoOut(

def repo: Repo =
Repo(owner.login, name)

def withBranch(branch: Branch): RepoOut =
copy(default_branch = branch, parent = parent.map(_.withBranch(branch)))

}

object RepoOut {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ class CliTest extends FunSuite {
List("--artifact-migrations", "/opt/scala-steward/extra-artifact-migrations.conf"),
List("--github-app-id", "12345678"),
List("--github-app-key-file", "example_app_key"),
List("--refresh-backoff-period", "1 day"),
List("--default-branch", "1.x")
List("--refresh-backoff-period", "1 day")
).flatten
)
val expected = Success(
Expand All @@ -51,8 +50,7 @@ class CliTest extends FunSuite {
artifactMigrations = List(uri"/opt/scala-steward/extra-artifact-migrations.conf"),
githubAppId = Some(12345678),
githubAppKeyFile = Some(File("example_app_key")),
refreshBackoffPeriod = 1.day,
defaultBranch = Some("1.x")
refreshBackoffPeriod = 1.day
)
)
assertEquals(obtained, expected)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,50 @@ import org.scalasteward.core.update.show
class gitTest extends ScalaCheckSuite {
test("commitMsgFor should work with static message") {
val commitsConfig = CommitsConfig(Some("Static message"))
forAll { update: Update => assertEquals(commitMsgFor(update, commitsConfig), "Static message") }
forAll { update: Update =>
assertEquals(commitMsgFor(update, commitsConfig, None), "Static message")
}
}

test("commitMsgFor adds branch if provided") {
val commitsConfig = CommitsConfig(Some(s"$${default}"))
val branch = Branch("some-branch")
forAll { update: Update =>
val expected = s"Update ${show.oneLiner(update)} to ${update.nextVersion} in ${branch.name}"
assertEquals(commitMsgFor(update, commitsConfig, Some(branch)), expected)
}
}

test("commitMsgFor should work with default message") {
val commitsConfig = CommitsConfig(Some("${default}"))
val commitsConfig = CommitsConfig(Some(s"$${default}"))
alejandrohdezma marked this conversation as resolved.
Show resolved Hide resolved
forAll { update: Update =>
val expected = s"Update ${show.oneLiner(update)} to ${update.nextVersion}"
assertEquals(commitMsgFor(update, commitsConfig), expected)
assertEquals(commitMsgFor(update, commitsConfig, None), expected)
}
}

test("commitMsgFor should work with templated message") {
val commitsConfig =
CommitsConfig(Some("Update ${artifactName} from ${currentVersion} to ${nextVersion}"))
CommitsConfig(Some(s"Update $${artifactName} from $${currentVersion} to $${nextVersion}"))
forAll { update: Update =>
val expected =
s"Update ${show.oneLiner(update)} from ${update.currentVersion} to ${update.nextVersion}"
assertEquals(commitMsgFor(update, commitsConfig), expected)
assertEquals(commitMsgFor(update, commitsConfig, None), expected)
}
}

test("commitMsgFor should work with templated message and non-default branch") {
val commitsConfig =
CommitsConfig(
Some(
s"Update $${artifactName} from $${currentVersion} to $${nextVersion} in $${branchName}"
)
)
val branch = Branch("some-branch")
forAll { update: Update =>
val expected =
s"Update ${show.oneLiner(update)} from ${update.currentVersion} to ${update.nextVersion} in ${branch.name}"
assertEquals(commitMsgFor(update, commitsConfig, Some(branch)), expected)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class VCSPackageTest extends FunSuite {
private val repo = Repo("foo", "bar")
private val update =
Update.Single("ch.qos.logback" % "logback-classic" % "1.2.0", Nel.of("1.2.3"))
private val updateBranch = git.branchFor(update)
private val updateBranch = git.branchFor(update, None)

test("listingBranch") {
assertEquals(listingBranch(GitHub, repo, updateBranch), s"foo/bar:${updateBranch.name}")
Expand Down
Loading