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

Clarification of "updatePullRequests" for grouped PRs #3318

Open
bcarter97 opened this issue Mar 18, 2024 · 4 comments
Open

Clarification of "updatePullRequests" for grouped PRs #3318

bcarter97 opened this issue Mar 18, 2024 · 4 comments

Comments

@bcarter97
Copy link
Contributor

When using updatePullRequests = "always", if Scala Steward PRs are up-to-date with the main branch then running Scala Steward will not push commits to these branches. This is what I would expect to happen. However, for the minor updates PR, Scala Steward will re-commit the changes regardless of already being up-to-date with the main branch.

Firstly, is it expected that using an updatePullRequests strategy of "always" should not update PRs that are up-to-date with main?

Secondly, if the above is true, should the grouped PRs follow this same convention? And if not, should there be a way to separately configure the updatePullRequests strategy of grouped dependencies?

The only reference I can find to something similar is this comment - #3051 (comment)

This seems to suggest that the commits should be applied again. However the behaviour may have changed since then that I can't find any documentation of.

@bcarter97
Copy link
Contributor Author

bcarter97 commented Apr 26, 2024

so from digging around it looks like the observed behaviour doesn't match the intention of the "always" strategy.

private def shouldBeUpdated(data: UpdateData): F[Boolean] = {
val result = gitAlg.isMerged(data.repo, data.updateBranch, data.baseBranch).flatMap {
case true => (false, "PR has been merged").pure[F]
case false =>
gitAlg.branchAuthors(data.repo, data.updateBranch, data.baseBranch).flatMap { authors =>
if (authors.length >= 2)
(false, s"PR has commits by ${authors.mkString(", ")}").pure[F]
else if (data.repoConfig.updatePullRequestsOrDefault === PullRequestUpdateStrategy.Always)
(true, "PR update strategy is set to always").pure[F]
else
gitAlg.hasConflicts(data.repo, data.updateBranch, data.baseBranch).map {
case true => (true, s"PR has conflicts with ${data.baseBranch.name}")
case false => (false, s"PR has no conflict with ${data.baseBranch.name}")
}
}
}
result.flatMap { case (update, msg) => logger.info(msg).as(update) }
}

This code suggests if the strategy is always it will update regardless of already being up to date, whereas we see updates only if the branch is out of date (except for the grouped PR branch which always updates).

This makes me wonder if there should be an update strategy for when you want ScalaSteward to always update if it's out of date with the main branch, and if its up to date skip, e.g. if-stale - thoughts?

@mzuehlke
Copy link
Member

Hi,
are you seeing this on a repo on Github.com ?
It could be related to not propperly caching the workspace: https://github.com/scala-steward-org/scala-steward/blob/main/docs/faq.md#why-doesnt-self-hosted-scala-steward-close-obsolete-prs
Then Scala Steward wouldn't know what is the sate of an existing pull-request.

For grouped updates the rules are slightly different.

@bcarter97
Copy link
Contributor Author

Yes, this is running on Github.com.
The workspace is persistent between runs, ScalaSteward closes old PRs.

For grouped updates the rules are slightly different

In what way?

@mzuehlke
Copy link
Member

thanks I don't know the logic. I will try to understand and come back...

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

No branches or pull requests

2 participants