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 automatic media job type compatibility and introduce the DataVertical enum #1151

Merged
merged 2 commits into from
Sep 5, 2022

Conversation

incjo
Copy link
Contributor

@incjo incjo commented Aug 30, 2022

makes progress against #1065 by building on top of #1062 by further expanding support for automatically handling media jobs.

It allows not writing dedicated media adapters in cases where the existing photo/video functionality is satisfactory. It also provides automatic interoperability with destinations that only provide a Media adapter.

Main contributions:

  • replaced MediaToPhotoConversionImporter and MediaToPhotoConversionExporter with generic versions that can support conversion between any two adapters: AnyToAnyImporter and AnyToAnyExporter
  • added special case adapters MediaExporterDecorator and MediaImporterDecorator which combine existing photo and video adapters to seamlessly cover the media job type.
  • added a compatibility layer TransferCompatibilityProvider which attempts to build a substitute adapter upon not finding a real one. E.g. when a media job can't find a media importer, it will combine existing photo and video importers to perform the job.
  • added a DataVertical enum which replaces the hardcoded type strings like "PHOTOS".

Review the commit separately as the DataVertical commit is very monotonic.


note: this PR has a lot in it due to #1136 (comment)

@incjo incjo changed the title Add automatic media job type compatibility and introduce a DataVertical enum Add automatic media job type compatibility and introduce the DataVertical enum Aug 30, 2022
@jzacsh jzacsh self-requested a review August 30, 2022 19:03
Copy link
Collaborator

@jzacsh jzacsh left a comment

Choose a reason for hiding this comment

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

LGTM! I left a comment; also optional bit:

up to you, but this is easier to follow as two commits the way you have them, so when you click the merge button in github, consider unticking the "squash" button so that it keeps the commits separated (your commit to automate stuff; your commit to add an enum).

jzacsh
jzacsh previously approved these changes Aug 31, 2022
Copy link
Collaborator

@wmorland wmorland left a comment

Choose a reason for hiding this comment

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

👍

@wmorland wmorland merged commit 8a0e3e7 into dtinit:master Sep 5, 2022
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.

3 participants