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.

Sign up for GitHub

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

Jump to bottom

Refactor livecheck strategy block handling #11842

Merged
merged 6 commits into from
Aug 12, 2021
Merged

Refactor livecheck strategy block handling #11842

merged 6 commits into from
Aug 12, 2021

Commits on Aug 11, 2021

  1. Rework how strategy constants are identified 

    Up to this point, we've had to rely on making `Strategy` constants
private to ensure that the only available constants are strategies.
With the current setup, the existence of a constant that's not a
strategy would break `Strategy#strategies` and
`Livecheck#livecheck_strategy_names`.

Instead, we can achieve the same goal by skipping over constants
that aren't a class. Other than saving us from having to make these
constants private, this is necessary to be able to create a
`Strategy` constant that can be used in all strategies.
    @samford
    samford committed Aug 11, 2021
    Configuration menu
    Copy the full SHA
    cf79ced View commit details
    Browse the repository at this point in the history

  2. Standardize valid strategy block return types 

    Valid `strategy` block return types currently vary between
strategies. Some only accept a string whereas others accept a string
or array of strings. [`strategy` blocks also accept a `nil` return
(to simplify early returns) but this was already standardized across
strategies.]

While some strategies only identify one version by default (where a
string is an appropriate return type), it could be that a strategy
block identifies more than one version. In this situation, the
strategy would need to be modified to accept (and work with) an
array from a `strategy` block.

Rather than waiting for this to become a problem, this modifies all
strategies to standardize on allowing `strategy` blocks to return a
string or array of strings (even if only one of these is currently
used in practice). Standardizing valid return types helps to further
simplify the mental model for `strategy` blocks and reduce cognitive
load.

This commit extracts related logic from `#find_versions` into
methods like `#versions_from_content`, which is conceptually similar
to `PageMatch#page_matches` (renamed to `#versions_from_content`
for consistency). This allows us to write tests for the related code
without having to make network requests (or stub them) at this point.
In general, this also helps to better align the structure of
strategies and how the various `#find_versions` methods work with
versions.

There's still more planned work to be done here but this is a step
in the right direction.
    @samford
    samford committed Aug 11, 2021
    Configuration menu
    Copy the full SHA
    56dd891 View commit details
    Browse the repository at this point in the history

  3. Improve standardization of strategy tests

    @samford
    samford committed Aug 11, 2021
    Configuration menu
    Copy the full SHA
    7e07010 View commit details
    Browse the repository at this point in the history

  4. Add additional method signatures to strategies

    @samford
    samford committed Aug 11, 2021
    Configuration menu
    Copy the full SHA
    f2bd39c View commit details
    Browse the repository at this point in the history

  5. Improve documentation comments

    @samford
    samford committed Aug 11, 2021
    Configuration menu
    Copy the full SHA
    c936a94 View commit details
    Browse the repository at this point in the history

  6. Remove unnecessary require statements

    @samford
    samford committed Aug 11, 2021
    Configuration menu
    Copy the full SHA
    c59d5db View commit details
    Browse the repository at this point in the history