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

Refactor Valkyrie indexer API (to match forms) #6441

Merged
merged 5 commits into from
Nov 17, 2023
Merged

Conversation

marrus-sh
Copy link
Collaborator

@marrus-sh marrus-sh commented Nov 13, 2023

Note: Because there are some file renames which don’t persist across the full diff, I recommend reviewing each commit in sequence.

This is a kind of heavy refactor of the Valkyrie indexer classes to match the API used for forms. This means :⁠—

  • Using configurable classes, like Hyrax.config.pcdm_collection_indexer, instead of hardcoded ones.

  • Renaming all of the classes to follow a pattern and be namespaced to Hyrax::Indexers :⁠—

    • Hyrax::ValkyrieIndexer 🔜 Hyrax::Indexers::ResourceIndexer.

      Note: This is a somewhat less‐basic base class, as it includes Hyrax::ResourceIndexer. If we still want Hyrax::ValkyrieIndexer as a base class which does not utilize Hyrax::ResourceIndexer, that’s probably an option.

    • Hyrax::PcdmCollectionIndexer 🔜 Hyrax::Indexers::PcdmCollectionIndexer.

      Note: This class has already been renamed once in the past (it was originally Hyrax::ValkyrieCollectionIndexer; see 7ca32a5).

    • Hyrax::ValkyrieFileSetIndexer 🔜 Hyrax::Indexers::FileSetIndexer.

    • Hyrax::ValkyrieWorkIndexer 🔜 Hyrax::Indexers::PcdmObjectIndexer.

    The old aliases are supported with deprecation warnings.

  • Implementing Hyrax::Indexers::ResourceIndexer() and Hyrax::Indexers::PcdmObjectIndexer() to match Hyrax::Forms::ResourceForm() and Hyrax::Forms::PcdmObjectForm().

@marrus-sh marrus-sh force-pushed the refactor_indexers branch 5 times, most recently from 4af9a27 to e5fdc14 Compare November 16, 2023 00:23
@marrus-sh marrus-sh marked this pull request as ready for review November 16, 2023 17:32
@marrus-sh marrus-sh added notes-major Release Notes: Potentially breaking changes notes-valkyrie Release Notes: Valkyrie specific labels Nov 16, 2023
The `app/indexers/` directory is kind of messy, and it’s not clear what
is an indexer class and what is a module that indexers can include.
Moving these files clarifies the situation.
This is a kind of heavy refactor of the Valkyrie indexer classes to
match the API used for forms. This means :⁠—

- Using configurable classes, like
  `Hyrax.config.pcdm_collection_indexer`, instead of hardcoded ones.

- Renaming all of the classes to follow a pattern and be namespaced to
  `Hyrax::Indexers` :⁠—

  - `Hyrax::ValkyrieIndexer` 🔜 `Hyrax::Indexers::ResourceIndexer`.

    **Note:** This is a somewhat less‐basic base class, as it includes
    `Hyrax::ResourceIndexer`. If we still want `Hyrax::ValkyrieIndexer`
    as a base class which does not utilize `Hyrax::ResourceIndexer`,
    that’s probably an option.

  - `Hyrax::PcdmCollectionIndexer` 🔜
    `Hyrax::Indexers::PcdmCollectionIndexer`.

    **Note:** This class has already been renamed once in the past (it
    was originally `Hyrax::ValkyrieCollectionIndexer`; see
    7ca32a5).

  - `Hyrax::ValkyrieFileSetIndexer` 🔜 `Hyrax::Indexers::FileSetIndexer`.

  - `Hyrax::ValkyrieWorkIndexer` 🔜 `Hyrax::Indexers::PcdmObjectIndexer`.

  The old aliases are supported with deprecation warnings.

- Implementing `Hyrax::Indexers::ResourceIndexer()` and
  `Hyrax::Indexers::PcdmObjectIndexer()` to match
  `Hyrax::Forms::ResourceForm()` and `Hyrax::Forms::PcdmObjectForm()`.
This is essentially just a find‐and‐replace of
`Hyrax::Indexers::ResourceIndexer` in place of
`Hyrax::ValkyrieIndexer`
Copy link
Contributor

@dlpierce dlpierce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reorg! This is a welcome cleanup.

@dlpierce dlpierce merged commit 3ff1ca1 into main Nov 17, 2023
4 checks passed
@dlpierce dlpierce deleted the refactor_indexers branch November 17, 2023 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes-major Release Notes: Potentially breaking changes notes-valkyrie Release Notes: Valkyrie specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants