Generator refactor #121
Assignees
Labels
Comments
jeremyf
added a commit
that referenced
this issue
Feb 8, 2023
Prior to this commit, we injected code into the downstream `SolrDocument` via the install generator. With this commit, we eschew adding code via a generator and instead decoration during the engine `to_prepare` block. The benefit continues to be that we minimize the surface area of what we release downstream while we're still working on this gem/engine. Related to: - #121
jeremyf
added a commit
that referenced
this issue
Feb 9, 2023
Prior to this commit, we were carrying over the `first_page_bsi` field from the newspaper_works gem. Looking at that gem there is the following in index behavior: ```bash app/indexers/concerns/newspaper_works/indexes_relationships.rb 90: solr_doc['first_page_bsi'] = true if this_page_index.zero? ``` When I run `rg first_page` prior to this commit the only "hit" is found in the generator method that I'm removing. After this commit, there are no more references to `first_page_bsi`. Related to: - #121
jeremyf
added a commit
that referenced
this issue
Feb 9, 2023
Prior to this commit, we do two things: - Copy downstream a search builder class - Update the downstream configuration to use that search builder class With this commit, we no longer copy that search builder class downstream. Instead we let the configuration suffice. This means that downstream implementations can extend (e.g. inherit from the `IiifPrint::CatalogSearchBuilder`, use it as is, or ignore it). Regardless, we're not sending that code off on its own to then drift from our assumptive implementation; thus creating upgrade and maintenance issues. Related to: - #121
jeremyf
added a commit
that referenced
this issue
Feb 9, 2023
As a matter of practice, when you run an install generator you should
follow this high level script:
1. Stash all of your existing work.
2. Checkout main branch
3. Pull the main branch
4. Create a new branch from main
5. Run the generator
6. Review the changes
**tl;dr** - Make sure you can review what this generator did to the
files in your project.
The logic to conditionally delete a file is an unnecessary conditional
assuming the person running the generator follows the above process.
Related to:
- #121
jeremyf
added a commit
that referenced
this issue
Feb 10, 2023
The newspaper_works gem amended the advanced search to include date range searching. We brought that along as we cloned the repository to then begin the gem's adjustments. As date range searching is not an initial feature of IIIF Print, we can remove that. With that it does not make sense for IIIF Print to depend on blacklight advanced search. If a downstream implementation wants advanced search, they can look at how to incorporate that into their application. Related to: - #121
jeremyf
added a commit
that referenced
this issue
Feb 13, 2023
Prior to this commit, we generated code into the downstream application. This code included two injected modules: `SearchBehavior` and `AnnotationBehavior`. In the case of `SearchBehavior` we had switching logic based on models that came from the `newspaper_works` gem. With this commit, I removed the `SearchBehavior` installation. It is not something we need given the current constraints. And instead of installing a module to extend the `BlacklightIiifSearch::Annotation` via the "behavior" pattern, we're directly prepending a decorator to the class which included the `AnnotationBehavior` (e.g. cut out the middle module). Last I removed a commented out line; it was additional chatter we didn't need. Related to: - #121
jeremyf
added a commit
that referenced
this issue
Feb 13, 2023
Prior to this commit, we generated code into the downstream application. This code included two injected modules: `SearchBehavior` and `AnnotationBehavior`. In the case of `SearchBehavior` we had switching logic based on models that came from the `newspaper_works` gem. With this commit, I removed the `SearchBehavior` installation. It is not something we need given the current constraints. And instead of installing a module to extend the `BlacklightIiifSearch::Annotation` via the "behavior" pattern, we're directly prepending a decorator to the class which included the `AnnotationBehavior` (e.g. cut out the middle module). Last I removed a commented out line; it was additional chatter we didn't need. Related to: - #121
jeremyf
added a commit
that referenced
this issue
Feb 13, 2023
Prior to this commit, we had two generators touching the same file, with each generator doing one small thing. With this commit, I've consolidated some of the logic into a single generator. Closes #121
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The goal of this ticket is to minimize the install generator's impact on downstream applications. That is to not copy in lots of code, but instead keep it within the gem.
The text was updated successfully, but these errors were encountered: