add requirement and detection that breaking changes are explained in release notes (#586)

* add requirement and detection that breaking changes are explained in release notes

* add unit tests

* fix get_release_notes

* look up the PR at guideline check time to protect against slow labelling

* Update src/AutoMerge/guidelines.jl

Co-authored-by: Eric Hanson <[email protected]>

* accommodate unit tests

* make it opt-in in `run`

* tryfix

* fix

* expand message with example

* indent?

* refactor and fix indentation

* update reference tests

* add more details and include JuliaHub

* Update Project.toml

---------

Co-authored-by: Eric Hanson <[email protected]>

Author: IanButterworth

Project.toml

@@ -1,7 +1,7 @@
 name = "RegistryCI"
 uuid = "0c95cc5f-2f7e-43fe-82dd-79dbcba86b32"
 authors = ["Dilum Aluthge <[email protected]>", "Fredrik Ekre <[email protected]>", "contributors"]
-version = "10.9.1"
+version = "10.10.0"
 
 [deps]
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"

docs/src/guidelines.md

@@ -20,6 +20,7 @@ function guidelines_to_markdown_output(guidelines_function::Function)
     guidelines = guidelines_function(
         registration_type;
         check_license = true,
+        check_breaking_explanation = true,
         this_is_jll_package = false,
         this_pr_can_use_special_jll_exceptions = false,
         use_distance_check = false,
@@ -50,6 +51,7 @@ function guidelines_to_markdown_output(guidelines_function::Function)
     guidelines = guidelines_function(
         registration_type;
         check_license = true,
+        check_breaking_explanation = true,
         this_is_jll_package = false,
         this_pr_can_use_special_jll_exceptions = false,
         use_distance_check = false,
@@ -130,6 +132,28 @@ very possible for a perfectly good name to not pass the automatic checks and
 require manual merging. They simply exist to provide a fast path so that
 manual review is not required for every new package.
 
+### Providing and updating release notes
+
+When invoking a registration with the `@JuliaRegister` bot, release notes can be added in the form
+```
+@JuliaRegistrator register
+
+Release notes:
+
+## Breaking changes
+
+- Explanation of breaking change, ideally with upgrade tips
+- ...
+```
+
+These can also be added/updated on the General PR by re-invoking with the above.
+
+Doing this has two benefits:
+ - helps explanations during the registration process, especially for breaking changes
+ - release notes are picked up by TagBot such that they are added to the new release on the orignial repo
+
+Automerge is disabled for breaking changes where release notes are not provided mentioning "breaking".
+
 ## List of all GitHub PR labels that can influence AutoMerge
 
 AutoMerge reads certain labels on GitHub registration pull requests to influence its decisions.

docs/src/private-registries.md

@@ -73,6 +73,7 @@ RegistryCI.AutoMerge.run(
     additional_statuses = String[],
     additional_check_runs = String[],
     check_license = true,
+    check_breaking_explanation = true,
     public_registries = String["https://github.com/HolyLab/HolyLabRegistry"],
 )
 ```

example_github_workflow_files/automerge.yml

@@ -35,4 +35,4 @@ jobs:
           AUTOMERGE_GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           AUTOMERGE_TAGBOT_TOKEN: ${{ secrets.TAGBOT_TOKEN }}
           JULIA_DEBUG: RegistryCI,AutoMerge
-        run: julia --project=.ci/ -e 'using RegistryCI; using Dates; RegistryCI.AutoMerge.run(merge_new_packages = true, merge_new_versions = true, new_package_waiting_period = Day(3), new_jll_package_waiting_period = Minute(15), new_version_waiting_period = Minute(15), new_jll_version_waiting_period = Minute(15), registry = "JuliaRegistries/General", tagbot_enabled=true, authorized_authors = String["JuliaRegistrator"], authorized_authors_special_jll_exceptions = String["jlbuild"], suggest_onepointzero = false, additional_statuses = String[], additional_check_runs = String["Travis CI - Pull Request"])'
+        run: julia --project=.ci/ -e 'using RegistryCI; using Dates; RegistryCI.AutoMerge.run(merge_new_packages = true, merge_new_versions = true, new_package_waiting_period = Day(3), new_jll_package_waiting_period = Minute(15), new_version_waiting_period = Minute(15), new_jll_version_waiting_period = Minute(15), registry = "JuliaRegistries/General", tagbot_enabled=true, authorized_authors = String["JuliaRegistrator"], authorized_authors_special_jll_exceptions = String["jlbuild"], suggest_onepointzero = false, additional_statuses = String[], additional_check_runs = String["Travis CI - Pull Request"], check_breaking_explanation = true)'

src/AutoMerge/guidelines.jl

@@ -322,6 +322,74 @@ function meets_name_match_check(
     return (true, "")
 end
 
+# This check checks for an explanation of why a breaking change is breaking
+const guideline_breaking_explanation = Guideline(;
+    info = "Release notes have not been provided that explain why this is a breaking change.",
+    docs = "If this is a breaking change, release notes must be given that explain why this is a breaking change (i.e. mention \"breaking\"). To update the release notes, please see the \"Providing and updating release notes\" subsection under \"Additional information\" below.",
+    check=data -> meets_breaking_explanation_check(data))
+
+function meets_breaking_explanation_check(data::GitHubAutoMergeData)
+    # Look up PR here in case the labels are slow to be applied by the Registrator bot
+    # which decides whether to add the BREAKING label
+    pr = GitHub.pull_request(data.api, data.registry, data.pr.number; auth=data.auth)
+    return meets_breaking_explanation_check(pr.labels, pr.body)
+end
+
+function breaking_explanation_message(has_release_notes)
+    example_detail = """
+        <details><summary>Example of adding release notes with breaking notice</summary>
+
+        If you are using the comment bot `@JuliaRegistrator`, you can add release notes to this registration by re-triggering registration while specifying release notes:
+
+        ```
+        @JuliaRegistrator register
+
+        Release notes:
+
+        ## Breaking changes
+
+        - Explanation of breaking change, ideally with upgrade tips
+        - ...
+        ```
+
+        If you are using JuliaHub, trigger registration the same way you did the first time, but enter release notes that specify the breaking changes.
+        </details>
+    """
+    if has_release_notes
+        return """
+        This is a breaking change, but no release notes have been provided. Please add release notes that explain the breaking change.
+        $(example_detail)
+        """
+    else
+        return """
+        This is a breaking change, but the release notes do not mention it. Please add a mention of the breaking change to the release notes.
+        $(example_detail)
+        """
+    end
+end
+
+function meets_breaking_explanation_check(labels::Vector, body::AbstractString)
+    if any(==("BREAKING"), labels)
+        release_notes = get_release_notes(body)
+        if release_notes === nothing
+            msg = breaking_explanation_message(false)
+            return false, msg
+        elseif !occursin(r"breaking", lowercase(release_notes))
+            msg = breaking_explanation_message(true)
+            return false, msg
+        else
+            return true, ""
+        end
+    else
+        return true, ""
+    end
+end
+
+function get_release_notes(body::AbstractString)
+    pattern = r"<!-- BEGIN RELEASE NOTES -->(?s)(.*?)<!-- END RELEASE NOTES -->"
+    m = match(pattern, body)
+    return m === nothing ? nothing : m.captures[1]
+end
 
 # This check looks for similar (but not exactly matching) names. It can be
 # overridden by a label.
@@ -1078,7 +1146,8 @@ function get_automerge_guidelines(
     this_is_jll_package::Bool,
     this_pr_can_use_special_jll_exceptions::Bool,
     use_distance_check::Bool,
-    package_author_approved::Bool # currently unused for new packages
+    package_author_approved::Bool, # currently unused for new packages
+    check_breaking_explanation::Bool # not valid for new packages
 )
     guidelines = [
         # We first verify the name is a valid Julia identifier.
@@ -1124,6 +1193,7 @@ end
 function get_automerge_guidelines(
     ::NewVersion;
     check_license::Bool,
+    check_breaking_explanation::Bool,
     this_is_jll_package::Bool,
     this_pr_can_use_special_jll_exceptions::Bool,
     use_distance_check::Bool, # unused for new versions
@@ -1151,6 +1221,7 @@ function get_automerge_guidelines(
         (guideline_version_has_osi_license, check_license),
         (guideline_src_names_OK, true),
         (guideline_version_can_be_imported, true),
+        (guideline_breaking_explanation, check_breaking_explanation && !this_is_jll_package),
     ]
     return guidelines
 end

src/AutoMerge/public.jl

@@ -31,6 +31,7 @@ Run the `RegistryCI.AutoMerge` service.
 - `registry_deps`: list of registry dependencies, e.g your packages may depend on `General`.
 - `api_url`: the registry host API URL, default is `"https://api.github.com"`.
 - `check_license`: check package has a valid license, default is `false`.
+- `check_breaking_explanation`: Check whether the PR has release notes (collected via Registrator.jl) with a breaking change explanation, default is `false`.
 - `public_registries`: If a new package registration has a UUID that matches
    that of a package already registered in one of these registries supplied here
    (and has either a different name or different URL) then an error will be thrown.
@@ -62,6 +63,7 @@ RegistryCI.AutoMerge.run(
     additional_statuses = String[],
     additional_check_runs = String[],
     check_license = true,
+    check_breaking_explanation = true,
     public_registries = String["https://github.com/HolyLab/HolyLabRegistry"],
 )
 ```
@@ -95,6 +97,7 @@ function run(;
     registry_deps::Vector{<:AbstractString}=String[],
     api_url::String="https://api.github.com",
     check_license::Bool=false,
+    check_breaking_explanation::Bool=false,
     # A list of public Julia registries (repository URLs)
     # which will be checked for UUID collisions in order to
     # mitigate the dependency confusion vulnerability. See
@@ -158,6 +161,7 @@ function run(;
             whoami=whoami,
             registry_deps=registry_deps,
             check_license=check_license,
+            check_breaking_explanation=check_breaking_explanation,
             public_registries=public_registries,
             read_only=read_only,
             environment_variables_to_pass=environment_variables_to_pass,

src/AutoMerge/pull_requests.jl

@@ -86,6 +86,7 @@ function pull_request_build(
     point_to_slack::Bool,
     registry_deps::Vector{<:AbstractString}=String[],
     check_license::Bool,
+    check_breaking_explanation::Bool,
     public_registries::Vector{<:AbstractString}=String[],
     read_only::Bool,
     environment_variables_to_pass::Vector{<:AbstractString}=String[],
@@ -166,12 +167,12 @@ function pull_request_build(
         read_only=read_only,
         environment_variables_to_pass=environment_variables_to_pass,
     )
-    pull_request_build(data; check_license=check_license, new_package_waiting_period=new_package_waiting_period)
+    pull_request_build(data; check_license=check_license, check_breaking_explanation=check_breaking_explanation, new_package_waiting_period=new_package_waiting_period)
     rm(registry_master; force=true, recursive=true)
     return nothing
 end
 
-function pull_request_build(data::GitHubAutoMergeData; check_license, new_package_waiting_period)::Nothing
+function pull_request_build(data::GitHubAutoMergeData; check_license, check_breaking_explanation, new_package_waiting_period)::Nothing
     kind = package_or_version(data.registration_type)
     this_is_jll_package = is_jll_name(data.pkg)
     @info(
@@ -194,6 +195,7 @@ function pull_request_build(data::GitHubAutoMergeData; check_license, new_packag
     guidelines = get_automerge_guidelines(
         data.registration_type;
         check_license=check_license,
+        check_breaking_explanation=check_breaking_explanation,
         this_is_jll_package=this_is_jll_package,
         this_pr_can_use_special_jll_exceptions=this_pr_can_use_special_jll_exceptions,
         use_distance_check=perform_distance_check(data.pr.labels),

test/automerge-unit.jl

@@ -70,6 +70,8 @@ function comment_reference_test()
                 "_version_", version, "_point_to_slack_", point_to_slack)
                 reasons = [
                             AutoMerge.compat_violation_message(["julia"]),
+                            AutoMerge.breaking_explanation_message(true),
+                            AutoMerge.breaking_explanation_message(false),
                             "Example guideline failed. Please fix it."]
                 fail_text = AutoMerge.comment_text_fail(type, reasons, suggest_onepointzero, version; point_to_slack=point_to_slack)
 
@@ -321,6 +323,35 @@ end
         @test !AutoMerge.range_did_not_narrow(r2, r3)[1]
         @test !AutoMerge.range_did_not_narrow(r3, r2)[1]
     end
+
+    @testset "Breaking change releases must have explanatory release notes" begin
+        body_good = """
+        <!-- BEGIN RELEASE NOTES -->
+        `````
+        ## Breaking changes
+        - something
+        - something else
+        `````
+        <!-- END RELEASE NOTES -->
+        """
+        body_bad = """
+        <!-- BEGIN RELEASE NOTES -->
+        `````
+        ## Features
+        - something
+        - something else
+        `````
+        <!-- END RELEASE NOTES -->
+        """
+        body_bad_no_notes = ""
+
+        @test AutoMerge.meets_breaking_explanation_check(["BREAKING"], body_good)[1]
+        @test !AutoMerge.meets_breaking_explanation_check(["BREAKING"], body_bad)[1]
+        @test !AutoMerge.meets_breaking_explanation_check(["BREAKING"], body_bad_no_notes)[1]
+
+        # Maybe this should fail as the label isn't applied by JuliaRegistrator, so the version isn't breaking?
+        @test AutoMerge.meets_breaking_explanation_check([], body_good)[1]
+    end
 end
 
 @testset "Guidelines for both new packages and new versions" begin

test/reference_comments/comment_pass_false_type_new_package_suggest_onepointzero_false_version_0.1.0_point_to_slack_false.md

@@ -29,6 +29,46 @@ Please make sure that you have read the [package naming guidelines](https://juli
 
     </details>
 
+- This is a breaking change, but no release notes have been provided. Please add release notes that explain the breaking change.
+    <details><summary>Example of adding release notes with breaking notice</summary>
+
+    If you are using the comment bot `@JuliaRegistrator`, you can add release notes to this registration by re-triggering registration while specifying release notes:
+
+    ```
+    @JuliaRegistrator register
+
+    Release notes:
+
+    ## Breaking changes
+
+    - Explanation of breaking change, ideally with upgrade tips
+    - ...
+    ```
+
+    If you are using JuliaHub, trigger registration the same way you did the first time, but enter release notes that specify the breaking changes.
+    </details>
+
+
+- This is a breaking change, but the release notes do not mention it. Please add a mention of the breaking change to the release notes.
+    <details><summary>Example of adding release notes with breaking notice</summary>
+
+    If you are using the comment bot `@JuliaRegistrator`, you can add release notes to this registration by re-triggering registration while specifying release notes:
+
+    ```
+    @JuliaRegistrator register
+
+    Release notes:
+
+    ## Breaking changes
+
+    - Explanation of breaking change, ideally with upgrade tips
+    - ...
+    ```
+
+    If you are using JuliaHub, trigger registration the same way you did the first time, but enter release notes that specify the breaking changes.
+    </details>
+
+
 - Example guideline failed. Please fix it.
 
 ## 3. *Needs action*: here's what to do next

test/reference_comments/comment_pass_false_type_new_package_suggest_onepointzero_false_version_0.1.0_point_to_slack_true.md

@@ -29,6 +29,46 @@ Please make sure that you have read the [package naming guidelines](https://juli
 
     </details>
 
+- This is a breaking change, but no release notes have been provided. Please add release notes that explain the breaking change.
+    <details><summary>Example of adding release notes with breaking notice</summary>
+
+    If you are using the comment bot `@JuliaRegistrator`, you can add release notes to this registration by re-triggering registration while specifying release notes:
+
+    ```
+    @JuliaRegistrator register
+
+    Release notes:
+
+    ## Breaking changes
+
+    - Explanation of breaking change, ideally with upgrade tips
+    - ...
+    ```
+
+    If you are using JuliaHub, trigger registration the same way you did the first time, but enter release notes that specify the breaking changes.
+    </details>
+
+
+- This is a breaking change, but the release notes do not mention it. Please add a mention of the breaking change to the release notes.
+    <details><summary>Example of adding release notes with breaking notice</summary>
+
+    If you are using the comment bot `@JuliaRegistrator`, you can add release notes to this registration by re-triggering registration while specifying release notes:
+
+    ```
+    @JuliaRegistrator register
+
+    Release notes:
+
+    ## Breaking changes
+
+    - Explanation of breaking change, ideally with upgrade tips
+    - ...
+    ```
+
+    If you are using JuliaHub, trigger registration the same way you did the first time, but enter release notes that specify the breaking changes.
+    </details>
+
+
 - Example guideline failed. Please fix it.
 
 ## 3. *Needs action*: here's what to do next