From d759ebfe9340400b7db123cfba80c646f27e2057 Mon Sep 17 00:00:00 2001 From: MarkNahabedian Date: Fri, 28 Apr 2023 12:34:47 -0400 Subject: [PATCH] Introduce a parameter to contain Guideline parameters and plumb it through to each Guideline check! function so that each parameter doesn't need to be plumbed through separately. --- src/AutoMerge/cron.jl | 3 ++ src/AutoMerge/guidelines.jl | 52 ++++++++++++++++++++-------------- src/AutoMerge/public.jl | 5 ++++ src/AutoMerge/pull_requests.jl | 10 +++++-- src/AutoMerge/types.jl | 9 ++++-- 5 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/AutoMerge/cron.jl b/src/AutoMerge/cron.jl index 5d64d89e..132b806b 100644 --- a/src/AutoMerge/cron.jl +++ b/src/AutoMerge/cron.jl @@ -172,6 +172,7 @@ end function cron_or_api_build( api::GitHub.GitHubAPI, + guideline_parameters::GuidelineParameters, registry::GitHub.Repo; auth::GitHub.Authorization, authorized_authors::Vector{String}, @@ -202,6 +203,7 @@ function cron_or_api_build( my_retry() do cron_or_api_build( api, + guideline_parameters, pr, registry; auth=auth, @@ -239,6 +241,7 @@ end function cron_or_api_build( api::GitHub.GitHubAPI, + guideline_parameters::GuidelineParameters, pr::GitHub.PullRequest, registry::GitHub.Repo; auth::GitHub.Authorization, diff --git a/src/AutoMerge/guidelines.jl b/src/AutoMerge/guidelines.jl index a5b77a00..6a849024 100644 --- a/src/AutoMerge/guidelines.jl +++ b/src/AutoMerge/guidelines.jl @@ -3,7 +3,7 @@ using HTTP: HTTP const guideline_registry_consistency_tests_pass = Guideline(; info="Registy consistency tests", docs=nothing, - check=data -> + check=(data, guideline_parameters) -> meets_registry_consistency_tests_pass(data.registry_head, data.registry_deps), ) @@ -25,7 +25,8 @@ const guideline_compat_for_julia = Guideline(; "There is an upper-bounded `[compat]` entry for `julia` that ", "only includes a finite number of breaking releases of Julia.", ), - check=data -> meets_compat_for_julia(data.registry_head, data.pkg, data.version), + check=(data, guideline_parameters) -> + meets_compat_for_julia(data.registry_head, data.pkg, data.version), ) function meets_compat_for_julia(working_directory::AbstractString, pkg, version) @@ -73,7 +74,8 @@ const guideline_compat_for_all_deps = Guideline(; "are upper-bounded and only include a finite number of breaking releases. ", "For more information, please see the \"Upper-bounded `[compat]` entries\" subsection under \"Additional information\" below.", ), - check=data -> meets_compat_for_all_deps(data.registry_head, data.pkg, data.version), + check=(data, guideline_parameters) -> + meets_compat_for_all_deps(data.registry_head, data.pkg, data.version), ) function meets_compat_for_all_deps(working_directory::AbstractString, pkg, version) @@ -150,12 +152,13 @@ end const guideline_patch_release_does_not_narrow_julia_compat = Guideline(; info="If it is a patch release on a post-1.0 package, then it does not narrow the `[compat]` range for `julia`.", - check=data -> meets_patch_release_does_not_narrow_julia_compat( - data.pkg, - data.version; - registry_head=data.registry_head, - registry_master=data.registry_master, - ), + check=(data, guideline_parameters) -> + meets_patch_release_does_not_narrow_julia_compat( + data.pkg, + data.version; + registry_head=data.registry_head, + registry_master=data.registry_master, + ), ) function meets_patch_release_does_not_narrow_julia_compat( @@ -200,7 +203,7 @@ const _AUTOMERGE_NEW_PACKAGE_MINIMUM_NAME_LENGTH = 5 const guideline_name_length = Guideline(; info="Name not too short", docs="The name is at least $(_AUTOMERGE_NEW_PACKAGE_MINIMUM_NAME_LENGTH) characters long.", - check=data -> meets_name_length(data.pkg), + check=(data, guideline_parameters) -> meets_name_length(data.pkg), ) function meets_name_length(pkg) @@ -215,7 +218,7 @@ end const guideline_name_ascii = Guideline(; info="Name is composed of ASCII characters only.", - check=data -> meets_name_ascii(data.pkg), + check=(data, guideline_parameters) -> meets_name_ascii(data.pkg), ) function meets_name_ascii(pkg) @@ -228,7 +231,7 @@ end const guideline_julia_name_check = Guideline(; info="Name does not include \"julia\" or start with \"Ju\".", - check=data -> meets_julia_name_check(data.pkg), + check=(data, guideline_parameters) -> meets_julia_name_check(data.pkg), ) function meets_julia_name_check(pkg) @@ -263,7 +266,8 @@ To prevent confusion between similarly named packages, the names of new packages between the package name and any existing package must exceeds a certain a hand-chosen threshold (currently 2.5). """, - check=data -> meets_distance_check(data.pkg, data.registry_master), + check=(data, guideline_parameters) -> + meets_distance_check(data.pkg, data.registry_master), ) function meets_distance_check( @@ -377,7 +381,8 @@ const guideline_normal_capitalization = Guideline(; "contain only ASCII alphanumeric characters, ", "and contain at least one lowercase letter.", ), - check=data -> meets_normal_capitalization(data.pkg), + check=(data, guideline_parameters) -> + meets_normal_capitalization(data.pkg), ) function meets_normal_capitalization(pkg) @@ -392,7 +397,8 @@ end const guideline_repo_url_requirement = Guideline(; info="Repo URL ends with `/PackageName.jl.git`.", - check=data -> meets_repo_url_requirement(data.pkg; registry_head=data.registry_head), + check=(data, guideline_parameters) -> + meets_repo_url_requirement(data.pkg; registry_head=data.registry_head), ) function meets_repo_url_requirement(pkg::String; registry_head::String) @@ -446,7 +452,7 @@ const guideline_sequential_version_number = Guideline(; "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.", ), - check=data -> meets_sequential_version_number( + check=(data, guideline_parameters) -> meets_sequential_version_number( data.pkg, data.version; registry_head=data.registry_head, @@ -488,7 +494,8 @@ end const guideline_standard_initial_version_number = Guideline(; info="Standard initial version number. Must be one of: `0.0.1`, `0.1.0`, `1.0.0`, or `X.0.0`.", - check=data -> meets_standard_initial_version_number(data.version), + check=(data, guideline_parameters) -> + meets_standard_initial_version_number(data.version), ) function meets_standard_initial_version_number(version) @@ -540,7 +547,7 @@ end const guideline_code_can_be_downloaded = Guideline(; info="Code can be downloaded.", - check=data -> meets_code_can_be_downloaded( + check=(data, guideline_parameters) -> meets_code_can_be_downloaded( data.registry_head, data.pkg, data.version, @@ -609,7 +616,7 @@ end const guideline_src_names_OK = Guideline(; info="`src` files and directories names are OK", - check=data -> meets_src_names_ok(data.pkg_code_path), + check=(data, guideline_parameters) -> meets_src_names_ok(data.pkg_code_path), ) function meets_code_can_be_downloaded(registry_head, pkg, version, pr; pkg_code_path) @@ -662,7 +669,7 @@ is_valid_url(str::AbstractString) = !isempty(HTTP.URI(str).scheme) && isvalid(HT const guideline_version_can_be_pkg_added = Guideline(; info="Version can be `Pkg.add`ed", docs="Package installation: The package should be installable (`Pkg.add(\"PackageName\")`).", - check=data -> meets_version_can_be_pkg_added( + check=(data, guideline_parameters) -> meets_version_can_be_pkg_added( data.registry_head, data.pkg, data.version; @@ -731,7 +738,8 @@ const guideline_version_has_osi_license = Guideline(; "This check is required for the General registry. ", "For other registries, registry maintainers have the option to disable this check.", ), - check=data -> meets_version_has_osi_license(data.pkg; pkg_code_path=data.pkg_code_path), + check=(data, guideline_parameters) -> + meets_version_has_osi_license(data.pkg; pkg_code_path=data.pkg_code_path), ) function meets_version_has_osi_license(pkg::String; pkg_code_path) @@ -793,7 +801,7 @@ end const guideline_version_can_be_imported = Guideline(; info="Version can be `import`ed", docs="Package loading: The package should be loadable (`import PackageName`).", - check=data -> meets_version_can_be_imported( + check=(data, guideline_parameters) -> meets_version_can_be_imported( data.registry_head, data.pkg, data.version; diff --git a/src/AutoMerge/public.jl b/src/AutoMerge/public.jl index c1024733..2d5e16e3 100644 --- a/src/AutoMerge/public.jl +++ b/src/AutoMerge/public.jl @@ -100,6 +100,9 @@ function run(; read_only::Bool=false, environment_variables_to_pass::Vector{<:AbstractString}=String[], )::Nothing + # guideline_parameters is for parameters that are passed directly + # from run through to the various Guidelines without modification: + guideline_parameters = GuidelineParameters() all_statuses = deepcopy(additional_statuses) all_check_runs = deepcopy(additional_check_runs) push!(all_statuses, "automerge/decision") @@ -140,6 +143,7 @@ function run(; pr_head_commit_sha = current_pr_head_commit_sha(cicfg; env=env) pull_request_build( api, + guideline_parameters, pr_number, pr_head_commit_sha, registry_repo, @@ -162,6 +166,7 @@ function run(; always_assert(run_merge_build) cron_or_api_build( api, + guideline_parameters, registry_repo; auth=auth, authorized_authors=authorized_authors, diff --git a/src/AutoMerge/pull_requests.jl b/src/AutoMerge/pull_requests.jl index 92671bc7..d6a8033a 100644 --- a/src/AutoMerge/pull_requests.jl +++ b/src/AutoMerge/pull_requests.jl @@ -71,6 +71,7 @@ end function pull_request_build( api::GitHub.GitHubAPI, + guideline_parameters::GuidelineParameters, pr_number::Integer, current_pr_head_commit_sha::String, registry::GitHub.Repo, @@ -163,12 +164,15 @@ function pull_request_build( read_only=read_only, environment_variables_to_pass=environment_variables_to_pass, ) - pull_request_build(data; check_license=check_license) + pull_request_build(data, guideline_parameters; + check_license=check_license) rm(registry_master; force=true, recursive=true) return nothing end -function pull_request_build(data::GitHubAutoMergeData; check_license)::Nothing +function pull_request_build(data::GitHubAutoMergeData, + guideline_parameters::GuidelineParameters; + check_license)::Nothing kind = package_or_version(data.registration_type) this_is_jll_package = is_jll_name(data.pkg) @info( @@ -208,7 +212,7 @@ function pull_request_build(data::GitHubAutoMergeData; check_license)::Nothing ) end else - check!(guideline, data) + check!(guideline, data, guideline_parameters) @info( guideline.info, meets_this_guideline = passed(guideline), diff --git a/src/AutoMerge/types.jl b/src/AutoMerge/types.jl index 7c95d41e..94419967 100644 --- a/src/AutoMerge/types.jl +++ b/src/AutoMerge/types.jl @@ -134,6 +134,11 @@ end passed(guideline::Guideline) = guideline.passed message(guideline::Guideline) = guideline.message -function check!(guideline::Guideline, data::GitHubAutoMergeData) - return guideline.passed, guideline.message = guideline.check(data) + +const GuidelineParameters = Dict{Symbol, Any} + +function check!(guideline::Guideline, data::GitHubAutoMergeData, + guideline_parameters::GuidelineParameters) + return guideline.passed, guideline.message = + guideline.check(data, guideline_parameters) end