Skip to content

Commit

Permalink
AutoMerge: Allow skipping sequential versions with author-approval cr…
Browse files Browse the repository at this point in the history
…iteria (#539)

* respect `Override AutoMerge: name similarity is okay` labels

* update docs & example workflow

* only trigger on the correct label

* add kwargs to docs

* allow skipping sequential version criteria with author approval mechanism

* update `pr_comment_is_blocking`

* fix docs

* Apply suggestions from code review

Co-authored-by: Dilum Aluthge <[email protected]>

* Apply suggestions from code review

Co-authored-by: Dilum Aluthge <[email protected]>

* Update test/automerge-unit.jl

* Apply suggestions from code review

Co-authored-by: Dilum Aluthge <[email protected]>

* Update example_github_workflow_files/automerge.yml

Co-authored-by: Dilum Aluthge <[email protected]>

* Apply suggestions from code review

Co-authored-by: Dilum Aluthge <[email protected]>

---------

Co-authored-by: Dilum Aluthge <[email protected]>
  • Loading branch information
ericphanson and DilumAluthge authored Jan 29, 2024
1 parent 0299b4e commit 01458cf
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 6 deletions.
2 changes: 2 additions & 0 deletions docs/src/guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function guidelines_to_markdown_output(guidelines_function::Function)
this_is_jll_package = false,
this_pr_can_use_special_jll_exceptions = false,
use_distance_check = false,
package_author_approved = false,
)
filter!(x -> x[1] != :update_status, guidelines)
filter!(x -> !(x[1].docs isa Nothing), guidelines)
Expand Down Expand Up @@ -52,6 +53,7 @@ function guidelines_to_markdown_output(guidelines_function::Function)
this_is_jll_package = false,
this_pr_can_use_special_jll_exceptions = false,
use_distance_check = false,
package_author_approved = false,
)
filter!(x -> x[1] != :update_status, guidelines)
filter!(x -> !(x[1].docs isa Nothing), guidelines)
Expand Down
4 changes: 4 additions & 0 deletions docs/src/private-registries.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,7 @@ In that case you will also have to add the following
+ - run: chmod +x .ci/instantiate.sh
```
in `ci.yml` and also `TagBotTriggers.yml` and `automerge.yml` (in which the above appears twice) files if those features are used.

## Author approval workflow support

Some guidelines allow the person invoking registration (typically the package author) to "approve" AutoMerge even if the guideline is not passing. This is facilitated by a labelling workflow `author_approval.yml` that must run on the registry in order to translate author-approval comments into labels that AutoMerge can use. The [General registry's workflows](https://github.com/JuliaRegistries/General/tree/master/.github/workflows) should once again be used as an example.
2 changes: 1 addition & 1 deletion example_github_workflow_files/automerge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
# label is one that affects the execution of the workflow
# Note: since the label contains a colon, we need to use a workaround like https://github.com/actions/runner/issues/1019#issuecomment-810482716
# for the syntax to parse correctly.
if: "${{ github.event.action != 'labeled' || (github.event.action == 'labeled' && github.event.label.name == 'Override AutoMerge: name similarity is okay') }}"
if: "${{ github.event.action != 'labeled' || (github.event.action == 'labeled' && (github.event.label.name == 'Override AutoMerge: name similarity is okay' || github.event.label.name == 'Override AutoMerge: package author approved')) }}"
runs-on: ${{ matrix.os }}
strategy:
matrix:
Expand Down
6 changes: 5 additions & 1 deletion src/AutoMerge/cron.jl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ function all_specified_check_runs_passed(
return all(values(check_passed))
end

pr_comment_is_blocking(c::GitHub.Comment) = !occursin("[noblock]", body(c))
function pr_comment_is_blocking(c::GitHub.Comment)
# Note: `[merge approved]` is not case sensitive, to match the semantics of `contains` on GitHub Actions
not_blocking = occursin("[noblock]", body(c)) || occursin("[merge approved]", lowercase(body(c)))
return !not_blocking
end

function pr_has_no_blocking_comments(
api::GitHub.GitHubAPI,
Expand Down
13 changes: 10 additions & 3 deletions src/AutoMerge/guidelines.jl
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,10 @@ function _valid_change(old_version::VersionNumber, new_version::VersionNumber)
end
end

const PACKAGE_AUTHOR_APPROVAL_INSTRUCTIONS = string(
"**If this was not a mistake and you wish to merge this PR anyway,",
"write a comment that says `[merge approved]`.**")

const guideline_sequential_version_number = Guideline(;
info="Sequential version number",
docs=string(
Expand All @@ -485,6 +489,7 @@ const guideline_sequential_version_number = Guideline(;
"valid new versions are `1.0.1`, `1.1.1`, `1.2.0` and `2.0.0`. ",
"Invalid new versions include `1.0.2` (skips `1.0.1`), ",
"`1.3.0` (skips `1.2.0`), `3.0.0` (skips `2.0.0`) etc.",
PACKAGE_AUTHOR_APPROVAL_INSTRUCTIONS,
),
check=data -> meets_sequential_version_number(
data.pkg,
Expand Down Expand Up @@ -990,7 +995,8 @@ function get_automerge_guidelines(
check_license::Bool,
this_is_jll_package::Bool,
this_pr_can_use_special_jll_exceptions::Bool,
use_distance_check::Bool
use_distance_check::Bool,
package_author_approved::Bool # currently unused for new packages
)
guidelines = [
(guideline_registry_consistency_tests_pass, true),
Expand Down Expand Up @@ -1032,12 +1038,13 @@ function get_automerge_guidelines(
check_license::Bool,
this_is_jll_package::Bool,
this_pr_can_use_special_jll_exceptions::Bool,
use_distance_check::Bool # unused for new versions
use_distance_check::Bool, # unused for new versions
package_author_approved::Bool,
)
guidelines = [
(guideline_registry_consistency_tests_pass, true),
(guideline_pr_only_changes_allowed_files, true),
(guideline_sequential_version_number, !this_pr_can_use_special_jll_exceptions),
(guideline_sequential_version_number, !this_pr_can_use_special_jll_exceptions && !package_author_approved),
(guideline_version_number_no_prerelease, true),
(guideline_version_number_no_build, !this_pr_can_use_special_jll_exceptions),
(guideline_compat_for_julia, true),
Expand Down
3 changes: 2 additions & 1 deletion src/AutoMerge/pull_requests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ function pull_request_build(data::GitHubAutoMergeData; check_license)::Nothing
check_license=check_license,
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)
use_distance_check=perform_distance_check(data.pr.labels),
package_author_approved=has_package_author_approved_label(data.pr.labels)
)
checked_guidelines = Guideline[]

Expand Down
16 changes: 16 additions & 0 deletions src/AutoMerge/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -302,3 +302,19 @@ function get_all_non_jll_package_names(registry_dir::AbstractString)
unique!(packages)
return packages
end

const PACKAGE_AUTHOR_APPROVED_LABEL = "Override AutoMerge: package author approved"

function has_package_author_approved_label(labels)
# No labels? Not approved
isnothing(labels) && return false
for label in labels
if label.name === PACKAGE_AUTHOR_APPROVED_LABEL
# found the approval
@debug "Found `$(PACKAGE_AUTHOR_APPROVED_LABEL)` label"
return true
end
end
# Did not find approval
return false
end
13 changes: 13 additions & 0 deletions test/automerge-unit.jl
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,19 @@ end
@test !AutoMerge.perform_distance_check([GitHub.Label(; name="Override AutoMerge: name similarity is okay")])
@test !AutoMerge.perform_distance_check([GitHub.Label(; name="hi"), GitHub.Label(; name="Override AutoMerge: name similarity is okay")])
end
@testset "has_author_approved_label" begin
@test !AutoMerge.has_package_author_approved_label(nothing)
@test !AutoMerge.has_package_author_approved_label([GitHub.Label(; name="hi")])
@test AutoMerge.has_package_author_approved_label([GitHub.Label(; name="Override AutoMerge: package author approved")])
@test AutoMerge.has_package_author_approved_label([GitHub.Label(; name="hi"), GitHub.Label(; name="Override AutoMerge: package author approved")])
end
@testset "pr_comment_is_blocking" begin
@test AutoMerge.pr_comment_is_blocking(GitHub.Comment(; body="hi"))
@test AutoMerge.pr_comment_is_blocking(GitHub.Comment(; body="block"))
@test !AutoMerge.pr_comment_is_blocking(GitHub.Comment(; body="[noblock]"))
@test !AutoMerge.pr_comment_is_blocking(GitHub.Comment(; body="[noblock]hi"))
@test !AutoMerge.pr_comment_is_blocking(GitHub.Comment(; body="[merge approved] abc"))
end
@testset "`get_all_non_jll_package_names`" begin
registry_path = joinpath(DEPOT_PATH[1], "registries", "General")
packages = AutoMerge.get_all_non_jll_package_names(registry_path)
Expand Down

0 comments on commit 01458cf

Please sign in to comment.