automate DTP's injection of auto-converters so extensions don't have to do this #1065
Labels
Comments
jzacsh
added a commit
to jzacsh/dtp
that referenced
this issue
May 11, 2022
jzacsh
added a commit
that referenced
this issue
Jun 22, 2022
…factorings changes (#1062) * fixes #1058: add MediaContainerResourceTest & more adds MediaContainerResourceTest and fixes bug said test newly reveals in automatic root album creation logic during transmogrification. nit: in nit-fixing doc bug (documentation was wrong on `MediaContainerResource#ensureRootAlbum`) also just lifts conditional logic being pushed down into helper function; taking liberty here with my opinion - that Single Responsibility (SOLID) would remove this complexity to begin with (`ensureRootAlbum` should not care about what a transmogrification is or what its config is). * noop(nit:docs): point TODO at a trackable issue number * noop(readability) run Goog style via clang-format * noop(new api) Media* APIs: generate lowlevel types adds MediaContainerResource (and accompanying `MediaAlbum` under the hood) static APIs that admit these classes just wrap or maintain 1:1 correspondence to Photo* types. Unfortunately only adds test coverage via the top-level usage (`MediaContainerResourceTest`) because there's no unit test coverage in place for MediaAlbum and I'm already spinning my wheels a bit adding missing test infra. But splitting some of the MediaAlbum==PhotoAlbum comparison unit test sanity-checking logic (into non-extant MediaAlbumTest.java) would be the correct thing to do here. motivation: this unblocks continued `MediaVertical` work without the need to have (for example) a `AcmecoPhotoExporter` manually forked copy-pasta style into `AcmecoMediaExporter` and then cause immediate techdebt for us (through code drift). longer-term: This change _does_ bring into question some of the intermediate type planning we're doing here, but we've determined this change is not going to be throw away work (eg: to unblock continued Media vertical expansion) while we revisit and discuss intermediate types a bit more. Those discussions will change _how_ this code is called but are very unlikely to _remove_ callers to this code. * WIP: experimental (not even compiling) draft of converter logic with some TODOs * WIP: nitfixes; cleanups & renames renames variables taht were backwards cleans up code since MediaContainer APIs[^1] were pulled out of this work stream to get committed in parallel. [^1]: commit 8963426 (of branch `kate-jon-pair-utils`) * WIP: fixes compilation errors with my generics setup * noop: revert accidental formatting during merge * noop(refactor) moves container copy logic into ExportResult; unfortunately there are no unit tests of this class * noop(refactor) clang-format & add javadoc * noop(refactor) pull ExportResult<...> construction out * noop(docs) point to a tracked TODO: #1065 * WIP: merge fix: bad merge kept both versions of the same code * adds Media* importer by conversion of Photo* one also some nitpicks on some javadoc. * fix: 2c2c4a3 = uncompiling copy-pasta * fix incorrect marking of Import auto-support for Media Co-authored-by: William Morland <wmorland@fb.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
for context, see the TODO on lines of
extensions/data-transfer/portability-data-transfer-microsoft/src/main/java/org/datatransferproject/transfer/microsoft/MicrosoftTransferExtension.javain https://github.com/google/data-transfer-project/pull/1062/files inlined below:The text was updated successfully, but these errors were encountered: