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

AutoMerge: Allow skipping sequential versions with author-approval criteria #539

Merged
merged 16 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
author_approved = false,
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
)
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,
author_approved = false,
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
)
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 GHA
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -461,6 +461,10 @@ function _valid_change(old_version::VersionNumber, new_version::VersionNumber)
end
end

const AUTHOR_APPROVAL_INSTRUCTIONS = string(
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
"**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 @@ -471,6 +475,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.",
AUTHOR_APPROVAL_INSTRUCTIONS,
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
),
check=data -> meets_sequential_version_number(
data.pkg,
Expand Down Expand Up @@ -976,7 +981,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,
author_approved::Bool # currently unused for new packages
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
)
guidelines = [
(guideline_registry_consistency_tests_pass, true),
Expand Down Expand Up @@ -1018,12 +1024,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
author_approved::Bool
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
)
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 && !author_approved),
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
(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),
author_approved=has_author_approved_label(data.pr.labels)
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
)
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 @@ -288,3 +288,19 @@ function get_all_non_jll_package_names(registry_dir::AbstractString)
unique!(packages)
return packages
end

const AUTHOR_APPROVED_LABEL = "Override AutoMerge: package author approved"
ericphanson marked this conversation as resolved.
Show resolved Hide resolved

function has_author_approved_label(labels)
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
# No labels? Not approved
isnothing(labels) && return false
for label in labels
if label.name === AUTHOR_APPROVED_LABEL
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
# found the approval
@debug "Found `$(AUTHOR_APPROVED_LABEL)` label"
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
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_author_approved_label(nothing)
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
@test !AutoMerge.has_author_approved_label([GitHub.Label(; name="hi")])
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
@test AutoMerge.has_author_approved_label([GitHub.Label(; name="Override AutoMerge: package author approved")])
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
@test AutoMerge.has_author_approved_label([GitHub.Label(; name="hi"), GitHub.Label(; name="Override AutoMerge: package author approved")])
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
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
Loading