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

Grouped PR description includes updates ignored via // scala-steward:off #3138

Open
samspills opened this issue Aug 4, 2023 · 2 comments
Open

Comments

@samspills
Copy link
Contributor

We have a shared .scala-steward.conf that defines PR groupings. Because it's shared, we've opted to use the comment syntax to ignore dependency upgrades when the ignore is repo-specific (the // scala-steward:off style).

This is going well, the grouped PRs are created and the ignored dependencies are being ignored as expected. The only issue is that the grouped PR description still includes the version bumps that we've ignored and which aren't actually updated in the PR.

@rtyley
Copy link
Contributor

rtyley commented Sep 15, 2023

I think I'm seeing something similar - in my case Scala Steward is generating a PR description that describes many updates (including a noisy Play upgrade):

image

...we haven't configured that Play upgrade to be ignored, but due to some kind of error, Scala Steward is not able to generate a commit performing that update - we see Unable to bump version for update:

  2023-09-14 09:29:13,181 INFO  Create branch update/non_aws
  2023-09-14 09:29:13,630 WARN  Unable to bump version for update com.typesafe.play:(filters-helpers, filters-helpers_2.13) : 2.8.19 -> 2.8.20
  2023-09-14 09:29:13,765 WARN  Unable to bump version for update com.typesafe.play:(play-ahc-ws, play-ahc-ws_2.13) : 2.8.19 -> 2.8.20
  2023-09-14 09:29:13,926 WARN  Unable to bump version for update com.typesafe.play:(play-akka-http-server, play-akka-http-server_2.13) : 2.8.19 -> 2.8.20
  2023-09-14 09:29:14,028 WARN  Unable to bump version for update com.typesafe.play:(play-docs, play-docs_2.13) : 2.8.19 -> 2.8.20
  2023-09-14 09:29:14,148 WARN  Unable to bump version for update com.typesafe.play:(play-logback, play-logback_2.13) : 2.8.19 -> 2.8.20
  2023-09-14 09:29:14,232 WARN  Unable to bump version for update com.typesafe.play:(play-server, play-server_2.13) : 2.8.19 -> 2.8.20
  2023-09-14 09:29:14,353 WARN  Unable to bump version for update com.typesafe.play:(play-test, play-test_2.13) : 2.8.19 -> 2.8.20
  2023-09-14 09:29:14,469 WARN  Unable to bump version for update com.typesafe.play:(twirl-api, twirl-api_2.13) : 1.5.1 -> 1.5.2
  2023-09-14 09:29:14,786 WARN  Unable to bump version for update org.scala-sbt:sbt-dependency-tree : 1.9.4 -> 1.9.5
  2023-09-14 09:29:14,795 INFO  Push 1 commit(s)
  2023-09-14 09:29:15,996 INFO  Create PR update/non_aws

...in fact, only one update makes it through to a commit (a bump of sbt version, not Play version):

image

I don't know why Scala Steward was unable to bump most of those versions, but that's a separate thing - for this issue, the problem is that the PR description is unnecessarily noisy - if the PR isn't going to apply those changes, it shouldn't clog up the PR description with them!

The pull request description is created by NewPullRequestData.from(), which is called by NurtureAlg.preparePullRequest():

image

NewPullRequestData.from() is passed two parameters of interest:

  • data: UpdateData - data.update will be of type Update.Grouped - the original set of Updates that were identified for this Group.
  • edits: List[EditAttempt] - each EditAttempt can relate to at most one Update (if the EditAttempt is a UpdateEdit) and the relevant commits for that Update - this gives us a clue as to whether an Update is actually present or not

def from(
data: UpdateData,
branchName: String,
edits: List[EditAttempt] = List.empty,

At the moment, the PR description is driven by data.update - but maybe it should be driven by collecting the Updates from edits instead.

@alejandrohdezma
Copy link
Member

Ouch! 😓 Sorry, missed this last month. Yeah, I've seen this problem in the past too and I believe that shouldn't be expected. Outside of grouping those PRs won't event exist so it doesn't make sense Steward is listing them. @rtyley your suggestion seems to be accurate, I think I'll be able to take a look at it next week. I'll keep you posted.

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

3 participants