rosiel commented 1 year ago

What does this Pull Request do?

Removes optional configs in favour of putting them in the Starter Site.

Related GitHub Issue: #18 Related to https://github.com/Islandora-Devops/islandora-starter-site/pull/88/files

What's new?

no longer add view modes, contexts, view displays, or EVA views.
Does this change add any new dependencies? No.
Does this change require any other modifications to be made to the repository (i.e. Regeneration activity, etc.)? No
Could this change impact execution of existing code? No

How should this be tested?

Follow the instructions to set up. Are they still satisfactory?

Documentation Status

Does this change existing behaviour that's currently documented? yes
Does this change require new pages or sections of documentation? no - included in PR
Who does this need to be documented for? - new site builders
Associated documentation pull request(s): or documentation issue

Additional Notes:

Any additional information that you think would be helpful when reviewing this PR.

Interested parties

Tag @alxp

alxp commented 1 year ago

I tested this on an existing site (not built with starter site) and uninstalling the movule, switching to the new branch, and re-enabling the module, causes the context that was enabled on the site to disappear.

I think removing the entity view displays and EVAs makes sense but I think we should leave the contexts and maybe also the view modes.

rosiel commented 1 year ago

Yes, the purpose of this PR is for the mirador module, which shouldn't have opinions about how you make use of contexts or view modes, can have its config set up by the starter site.

I'm not in favour of having stub config in the mirador module, when all this kind of config makes sense with the rest of similar config in the starter site. Our testing framework is not rugged enough for us to maintain:

Doing this config as part of the starter site
and also having a "sensible default" for non-islandora users who are installing mirador alone (which is still not possible, with the requirement on islandora_iiif which is still a submodule of islandora). This default would also function as a starting point for those islandora users who are late (or early) to the party and want to install mirador on an existing site. We would have a hard time making it make sense without the rest of the islandora setup, and it would be impossible to maintain two possibly different ways of doing things.