Show release notes URL from POM in PR body #2863

fthomas · 2022-12-28T22:52:38Z

This PR shows a release notes URL from the POM in the body of PRs.

Currently, Scala Steward uses the project.scm.url or project.url and tries to find release notes / change logs / version diffs from these URLs by constructing common URLs and checking if they exist. These are then added to the PRs.

With this change, Scala Steward also reads a release notes URL from a project.properties."info.releaseNotesUrl" element in the POM and shows it in the PR body. It allows library maintainers to have their release notes anywhere they want them to be and still have Scala Steward show them in PRs. Note that I just made up the name releaseNotesUrl for this feature.

This has been partly motivated by com-lihaoyi/requests-scala#122. The changes in CoursierAlg also allow to extract more metadata in the future to maybe implement the idea @lefou mentioned in #2708 (comment).

/ping @ckipp01 and @lefou since I mentioned this the other day on Discord.

Open questions:

Is there prior art of adding release notes or changelog URLs to the POM? Maybe we can reuse some convention of adding URLs to the POM that other projects already use.
If not, is it okay to use releaseNotesUrl or should the element name be prefixed with scalasteward. or something like that?

codecov · 2022-12-28T23:00:34Z

Codecov Report

Base: 86.97% // Head: 88.23% // Increases project coverage by +1.26% 🎉

Coverage data is based on head (b9b525a) compared to base (7e1c995).
Patch coverage: 94.11% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2863      +/-   ##
==========================================
+ Coverage   86.97%   88.23%   +1.26%     
==========================================
  Files         156      157       +1     
  Lines        2994     3018      +24     
  Branches      201      203       +2     
==========================================
+ Hits         2604     2663      +59     
+ Misses        390      355      -35

Impacted Files	Coverage Δ
...la/org/scalasteward/core/application/Context.scala	`68.42% <ø> (ø)`
...ala/org/scalasteward/core/nurture/NurtureAlg.scala	`18.66% <87.50%> (+18.66%)`	⬆️
...a/org/scalasteward/core/coursier/CoursierAlg.scala	`96.07% <94.28%> (+24.33%)`	⬆️
...calasteward/core/coursier/DependencyMetadata.scala	`100.00% <100.00%> (ø)`
...rc/main/scala/org/scalasteward/core/util/uri.scala	`100.00% <100.00%> (ø)`
.../scala/org/scalasteward/core/vcs/VCSExtraAlg.scala	`100.00% <100.00%> (ø)`
.../scala/org/scalasteward/core/data/UpdateData.scala	`100.00% <0.00%> (+50.00%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

ckipp01 · 2022-12-29T04:56:48Z

Is there prior art of adding release notes or changelog URLs to the POM? Maybe we can reuse some convention of adding URLs to the POM that other projects already use.

I dug around a bit to see if I could find anything and the only thing I found is that some release plugins will use this same convention. For example you can see this in a couple apache Maven projects that use properties.commons.releaseNotesLocation. Here are a couple examples:

If not, is it okay to use releaseNotesUrl or should the element name be prefixed with scalasteward. or something like that?

I like releaseNotesUrl. It's clear. I actually don't think we should prefix it with scalasteward. This might be a good opportunity to introduce something that various tools in theory could use. So I'm in favor of leaving in generic.

Thanks for diving into this @fthomas!

fthomas · 2022-12-29T09:34:46Z

I like releaseNotesUrl. It's clear. I actually don't think we should prefix it with scalasteward. This might be a good opportunity to introduce something that various tools in theory could use. So I'm in favor of leaving in generic.

I agree with that and also think that it could be less likely that people adopt that property if it is tied to Scala Steward. At least I wouldn't want to sprinkle my POMs with tool specific properties.

fthomas · 2022-12-29T09:51:08Z

Btw, I'd like to note that the idea of adding a release notes URL to the POM has been discussed before in the
"Coordinate compiler and tooling releases?" thread on the Scala Contributors forum although it was discussed for the purpose of delaying PRs until a library is announced.

fthomas · 2022-12-31T14:07:17Z

Just noticed that with this change #2541 proposed by @julienrf should be almost trivial to implement by adding a new versionScheme field to DependencyMetadata and using that in NewPullRequestData.

modules/core/src/test/scala/org/scalasteward/core/nurture/NurtureAlgTest.scala

fthomas · 2023-01-02T20:02:23Z

I just looked at the POM of Cats 2.9.0. It contains these properties:

<properties>
  <info.apiURL>https://www.javadoc.io/doc/org.typelevel/cats-docs_2.13/2.9.0/</info.apiURL>
  <info.versionScheme>early-semver</info.versionScheme>
</properties>

Both come from the corresponding sbt settings apiURL and versionScheme. I don't know if there is something special about the info prefix but sbt is using that for the settings it writes into the POM. I'm now tempted to imitate sbt here and to require the info prefix for releaseNotesUrl too.

fthomas · 2023-01-02T21:19:35Z

The last commit changed the key to info.releaseNotesUrl. Matching the key is done case-insensitive, so it will also match info.releaseNotesURL or any other capitalisation. The added NurtureAlgTest demonstrates that URLs are still displayed correctly for a simple update. As far as I'm concerned this is ready to be merged.

ckipp01 · 2023-01-03T06:48:05Z

The last commit changed the key to info.releaseNotesUrl. Matching the key is done case-insensitive, so it will also match info.releaseNotesURL or any other capitalisation. The added NurtureAlgTest demonstrates that URLs are still displayed correctly for a simple update. As far as I'm concerned this is ready to be merged.

Awesome. I don't want to steal your thunder so I didn't do it, but you might want to announce this new feature in https://contributors.scala-lang.org/ so that more people are aware of it. Basically all over the place, ha.

alejandrohdezma

👏🏼 👏🏼

mzuehlke

Would you update the reference to the new property in the PR description, there you are still referring to project.properties.releaseNotesUrl

modules/core/src/main/scala/org/scalasteward/core/coursier/CoursierAlg.scala

Duplicates could have happened if the URL from the POM was the same as one of the URLs constructed in possibleReleaseRelatedUrls.

fthomas · 2023-01-04T09:58:24Z

Update: See #2863 (comment) for a better way to add this property in sbt.

Btw, in sbt the info.releaseNotesUrl property can be added to the POM vie pomPostProcess:

pomPostProcess := { (node: xml.Node) =>
  val releaseNotesUrl =
    <info.releaseNotesUrl>https://www.scala-lang.org/blog/2022/11/07/scala-3.2.1-released.html</info.releaseNotesUrl>
  var updated = false
  val updatedChild = node.child.map {
    case e: xml.Elem if e.label == "properties" =>
      updated = true; e.copy(child = e.child :+ releaseNotesUrl)
    case n => n
  }
  node match {
    case e: xml.Elem if updated => e.copy(child = updatedChild)
    case e: xml.Elem => e.copy(child = e.child :+ <properties>{releaseNotesUrl}</properties>)
    case n           => n
  }
}

This adds the info.releaseNotesUrl to an existing properties element or adds the properties element with a sole info.releaseNotesUrl element if the former does not exist.

Not sure if there is an easier way to do it. It would be certainly nice if this wouldn't be so complicated.

lefou · 2023-01-04T10:05:25Z

In Mill, you can add the property as follows:

// in object foo extends PublishModule
  def publishProperties = super.publishProperties() ++ Map(
    "info.releaseNotesURL" -> "https://www.scala-lang.org/blog/2022/11/07/scala-3.2.1-released.html"
  )

julienrf · 2023-01-04T10:48:56Z

@fthomas I think there is a simpler way for sbt:

projectID ~= { id =>
  val releaseNotesURL =
    "info.releaseNotesURL" -> "https://www.scala-lang.org/blog/2022/11/07/scala-3.2.1-released.html"
  id.withExtraAttributes(id.extraAttributes + releaseNotesURL)
}

fthomas · 2023-01-04T10:58:23Z

Awesome, thanks @julienrf!

This is a follow-up to #2863. Before this change, the `info.releaseNotesUrl` property from the POM was ignored if `project.scm.url` and `project.url` were not set. With this change the `releaseNotesUrl` is added to the PR even if the other two are not set.

fthomas · 2023-01-07T10:32:08Z

Just announced this on Scala Contributors: https://contributors.scala-lang.org/t/add-release-notes-urls-to-your-poms/6059

Update: Here is a GitHub search of issues that mention info.releaseNotesUrl: https://github.com/search?q=info.releaseNotesUrl&type=issues

fthomas added 2 commits December 28, 2022 22:48

Show release notes URL from POM in PR body

23533f8

Test getMetadata with an empty list of resolvers

6ce91d5

Test getMetadata with a resolver with headers

73cc23c

Add DependencyMetadataTest

0572e73

Merge branch 'main' into topic/grab-release-notes-url-from-pom

c23ea90

fthomas added 3 commits December 31, 2022 15:09

Merge branch 'main' into topic/grab-release-notes-url-from-pom

b84a3bc

Merge branch 'main' into topic/grab-release-notes-url-from-pom

9089ba3

Extract preparePullRequest from createPullRequest and test it

6709e21

mzuehlke reviewed Jan 2, 2023

View reviewed changes

modules/core/src/test/scala/org/scalasteward/core/nurture/NurtureAlgTest.scala Show resolved Hide resolved

fthomas added 2 commits January 2, 2023 19:51

Show GH release notes and version diff in preparePullRequest test

65de10b

Fetch the parent POM if repoUrl is empty

762a82d

fthomas added 3 commits January 2, 2023 21:06

Match releaseNotesURL key case-insensitive

2b5477a

Use equalsIgnoreCase

1488235

Change the property key to info.releaseNotesUrl

3c0e5ec

fthomas added the enhancement New feature or request label Jan 2, 2023

fthomas added this to the 0.20.0 milestone Jan 2, 2023

alejandrohdezma approved these changes Jan 3, 2023

View reviewed changes

mzuehlke approved these changes Jan 3, 2023

View reviewed changes

modules/core/src/main/scala/org/scalasteward/core/coursier/CoursierAlg.scala Outdated Show resolved Hide resolved

fthomas added 3 commits January 3, 2023 13:30

Deduplicate releaseRelatedUrls

1bf03d1

Duplicates could have happened if the URL from the POM was the same as one of the URLs constructed in possibleReleaseRelatedUrls.

Change return type of getMetadata to F[DependencyMetadata]

7e0bec6

fmt

b9b525a

fthomas merged commit 2157fae into main Jan 3, 2023

fthomas deleted the topic/grab-release-notes-url-from-pom branch January 3, 2023 15:00

fthomas mentioned this pull request Jan 6, 2023

Use releaseNotesUrl even if repoUrl is empty #2890

Merged

mzuehlke mentioned this pull request Jan 15, 2023

Add "versionScheme" labels to pull-requests. #2925

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show release notes URL from POM in PR body #2863

Show release notes URL from POM in PR body #2863

fthomas commented Dec 28, 2022 •

edited

Loading

codecov bot commented Dec 28, 2022 •

edited

Loading

ckipp01 commented Dec 29, 2022

fthomas commented Dec 29, 2022

fthomas commented Dec 29, 2022

fthomas commented Dec 31, 2022

fthomas commented Jan 2, 2023 •

edited

Loading

fthomas commented Jan 2, 2023

ckipp01 commented Jan 3, 2023

alejandrohdezma left a comment

mzuehlke left a comment

fthomas commented Jan 4, 2023 •

edited

Loading

lefou commented Jan 4, 2023 •

edited

Loading

julienrf commented Jan 4, 2023 •

edited

Loading

fthomas commented Jan 4, 2023

fthomas commented Jan 7, 2023 •

edited

Loading

Show release notes URL from POM in PR body #2863

Show release notes URL from POM in PR body #2863

Conversation

fthomas commented Dec 28, 2022 • edited Loading

codecov bot commented Dec 28, 2022 • edited Loading

Codecov Report

ckipp01 commented Dec 29, 2022

fthomas commented Dec 29, 2022

fthomas commented Dec 29, 2022

fthomas commented Dec 31, 2022

fthomas commented Jan 2, 2023 • edited Loading

fthomas commented Jan 2, 2023

ckipp01 commented Jan 3, 2023

alejandrohdezma left a comment

Choose a reason for hiding this comment

mzuehlke left a comment

Choose a reason for hiding this comment

fthomas commented Jan 4, 2023 • edited Loading

lefou commented Jan 4, 2023 • edited Loading

julienrf commented Jan 4, 2023 • edited Loading

fthomas commented Jan 4, 2023

fthomas commented Jan 7, 2023 • edited Loading

fthomas commented Dec 28, 2022 •

edited

Loading

codecov bot commented Dec 28, 2022 •

edited

Loading

fthomas commented Jan 2, 2023 •

edited

Loading

fthomas commented Jan 4, 2023 •

edited

Loading

lefou commented Jan 4, 2023 •

edited

Loading

julienrf commented Jan 4, 2023 •

edited

Loading

fthomas commented Jan 7, 2023 •

edited

Loading