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

Add Create Strategies #173

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add Create Strategies #173

wants to merge 5 commits into from

Conversation

chrisvanrun
Copy link
Contributor

@chrisvanrun chrisvanrun commented Nov 15, 2024

Part of the large PR:

This PR adds creation strategies for creating interdependent objects that can be used for creating display sets and archive items. Strategies should return an item that can be used to finalize the strategy, the CIV strategies return a ComponentInterfaceValuePostRequest.

Only tests for the prep stage of the strategies have been added since testing the actual execution of the strategies is non-trivial and is heavily reliant on the involved integration tests. The strategies are a result of heavy testing and extraction from the helper functions on the original PR.

This PR implements the logic behind the following:

Allowing both str, and list[str]

Sourcing a CIV from one file no longer requires a one-lengthed list, the helper functions automagically handle both. This also applies to pathlib.Path.

RE-use of images / CIVs

Can provide a CIV or Image to the strategies and it will be re-used.

@chrisvanrun chrisvanrun requested a review from jmsmkn November 15, 2024 13:58
@jmsmkn
Copy link
Member

jmsmkn commented Nov 26, 2024

I'm aware of this but because it is non-pitch related and still large it has low priority for me, so keeps getting pushed back by higher priority things. It may still be a while.

@chrisvanrun
Copy link
Contributor Author

I'm aware of this but because it is non-pitch related and still large it has low priority for me, so keeps getting pushed back by higher priority things. It may still be a while.

Check, I thought as much! Given its size and low priority it makes perfect sense. The assignment was a subtle async poke ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants