RegistryCI.jl
RegistryCI.jl copied to clipboard
Cleanup proposal concerning ordering of Guidelines and their applicability
Motivation:
There are ordering constraints among some Guidelines. For example, guideline_distance_check
should be run last. Also, guideline_code_can_be_downloaded
should be checked before guideline_version_has_osi_license
or any other guideline that depends on the candidate project's source code being available.
This ordering is currently imposed by the two methods of get_automerge_guidelines
: one for NewPackage
and one for NewVersion
. I have verified that these two methods both present the guidelines they do present in the same order. Having two separate implementations of the ordering offers the risk that those orderings could accidentally differ in the future.
In addition to imposing an ordering, get_automerge_guidelines
also tests whether a given Guideline is applicable based implicitly on the new package verses new version distinction, and explicitly on conditions hard coded in those methods rather than encoded as part of the Guideline. These conditions are hard coded in each method -- another opportunutry for assidental inconsistency.
Guidelines have various parameters which control, for example, how strict they should be. These parameters are passed in from the client workflows as keyword arguments to RegistryCI.AutoMerge.run
. Currently, each of these must be plumbed through run
, pull_request_build
, and check!
to the Guidelines themselves. This is needlessly cumbersome. It looks like in the past, parameters for new Guidelines have been either passed directly as new arguments to these functions, or included in GitHubAutoMergeData
.
Perhaps GitHubAutoMergeData
should only contain values passed to and returned from the GitHub API. Isolating the GitHub API interactioon to a single data structure is necessary, but not sufficient to address issues like https://github.com/JuliaRegistries/RegistryCI.jl/issues/438 to use RegistryCI with non-GitHub workflows.
Guideline parameters should be passed cleanly, without modification from run
, down through to their respective Guidelines. https://github.com/JuliaRegistries/RegistryCI.jl/pull/492 introduces a Dict for this purpose. Perhaps this should be an immutable struct instead? Then we could be sure it doesn't change, but a new field would need to be added for each Guideline parameter.
Proposal:
-
The constructor for struct Guideline should append the Guideline to global variable ALL_GUIDELINES. This is implemented in https://github.com/JuliaRegistries/RegistryCI.jl/pull/492.
-
Guideline gets a slot whose value is a collection of types for which the Guideline is applicable (
NewPackage
,NewVersion
, or both). -
Guideline gets a new slot whose value is a function that determines Guideline applicability.
-
Any data needed by
get_automerge_guidelines
should be passed in throughGitHubAutoMergeData
or the Guideline parameters Dict rather than as ad-hoc arguments. -
A single method of
get_automerge_guidelines
should loop over ALL_GUIDELINES testing which are applicable based on 2 and 3 above. -
Move the definition of each guideline to a separate file named by its global variable name, in a guidelines subdirectory. Include guideline files in the order that they should be checked to control the order they appear in ALL_GUIDELINES.
Open Issues
How would we control when update_status
should be called? Currently a special token appears in the Guideline orderings returned by get_automerge_guidelines
. I don't understand the motivation for this.
Perhaps each Guideline could have a flag that determines if update_status
should be called once the Guideline is checked.