Skip to content

Conversation

@yigit
Copy link
Collaborator

@yigit yigit commented Jun 19, 2020

  • Move Fetcher factories into companion

Fetcher factories were global methods, which made them hard
to discover since IDE cannot easily auto-complete.

This PR moves them into the companion of Fetcher while also
making Fetcher a real interface instead of a typealias.

Even though it is a bit more code for the developer, now they
can easily discover how to create a Fetcher by typing Fetcher.

Fixes: #167

  • make rx methods start w/ from too for consistency

  • Rename fether factories to be more clear, hopefully :/

  • remove fetch method, use invoke instead

  • Make Fetcher.from the one that receives a suspend fun.
    Create Fetcher.fromFlow for the flowing version.

Rename both SourceOfTruth builder methods to . Rely on param names to disambiguate

  • use .of instead, this seems better to me.

We should probably get rid of StoreBuilder.from and make it
Store.builder()

  • fix jvm name for SourceOfTruth.of with flow function

  • fix RxSourceOfTruth name to match original class

  • specify bounds for FactoryFetcher

  • updates per PR review

  • update graph per SoT rename

  • update rxjava3 APIs as well

These appeared after i rebased, missed them completely.
Also fixed some tests, appearantly IJ parameter name refactor does
not always work

  • supress wrong unnecessary cast warning

without this, multicaster cannot resolve to the base StoreResponse type

  • upgade gradle, try to fix build by disabling caching

  • split subscribers

  • resubscribe

Co-authored-by: miken [email protected]

Please see our contributing guidelines (contributing.md) primarily make sure to sign our cla as we cannot accept code externally without a signed cla

https://opensource.dropbox.com/cla/

* Move Fetcher factories into companion

Fetcher factories were global methods, which made them hard
to discover since IDE cannot easily auto-complete.

This PR moves them into the companion of Fetcher while also
making Fetcher a real interface instead of a typealias.

Even though it is a bit more code for the developer, now they
can easily discover how to create a Fetcher by typing Fetcher.

Fixes: #167

* make rx methods start w/ from too for consistency

* Rename fether factories to be more clear, hopefully :/

* remove fetch method, use invoke instead

* Make Fetcher.from the one that receives a suspend fun.
Create Fetcher.fromFlow for the flowing version.

Rename both SourceOfTruth builder methods to . Rely on param names to disambiguate

* use .of instead, this seems better to me.

We should probably get rid of StoreBuilder.from and make it
Store.builder()

* fix jvm name for SourceOfTruth.of with flow function

* fix RxSourceOfTruth name to match original class

* specify bounds for FactoryFetcher

* updates per PR review

* update graph per SoT rename

* update rxjava3 APIs as well

These appeared after i rebased, missed them completely.
Also fixed some tests, appearantly IJ parameter name refactor does
not always work

* supress wrong unnecessary cast warning

without this, multicaster cannot resolve to the base StoreResponse type

* upgade gradle, try to fix build by disabling caching

* split subscribers

* resubscribe

Co-authored-by: miken <[email protected]>
@yigit yigit requested review from digitalbuddha and eyalgu June 19, 2020 03:53
@codecov
Copy link

codecov bot commented Jun 19, 2020

Codecov Report

❗ No coverage uploaded for pull request base (main@a5c29ff). Click here to learn what that means.
The diff coverage is 85.71%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #181   +/-   ##
=======================================
  Coverage        ?   86.01%           
  Complexity      ?      230           
=======================================
  Files           ?       54           
  Lines           ?      951           
  Branches        ?      149           
=======================================
  Hits            ?      818           
  Misses          ?       76           
  Partials        ?       57           
Impacted Files Coverage Δ Complexity Δ
.../android/external/store4/impl/FetcherController.kt 100.00% <ø> (ø) 6.00 <0.00> (?)
...src/main/kotlin/com/dropbox/store/rx2/RxFetcher.kt 50.00% <50.00%> (ø) 0.00 <0.00> (?)
...src/main/kotlin/com/dropbox/store/rx3/RxFetcher.kt 50.00% <50.00%> (ø) 0.00 <0.00> (?)
...in/kotlin/com/dropbox/store/rx2/RxSourceOfTruth.kt 70.00% <100.00%> (ø) 0.00 <0.00> (?)
...in/kotlin/com/dropbox/store/rx3/RxSourceOfTruth.kt 70.00% <100.00%> (ø) 0.00 <0.00> (?)
...ava/com/dropbox/android/external/store4/Fetcher.kt 100.00% <100.00%> (ø) 0.00 <0.00> (?)
...m/dropbox/android/external/store4/SourceOfTruth.kt 88.23% <100.00%> (ø) 0.00 <0.00> (?)
...otlin/com/dropbox/flow/multicast/ChannelManager.kt 85.36% <0.00%> (ø) 8.00% <0.00%> (?%)
.../java/com/dropbox/android/external/fs3/FSWriter.kt 100.00% <0.00%> (ø) 3.00% <0.00%> (?%)
.../android/external/fs3/FileSystemRecordPersister.kt 100.00% <0.00%> (ø) 4.00% <0.00%> (?%)
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5c29ff...1a954e4. Read the comment docs.

@digitalbuddha digitalbuddha merged commit b860504 into main Jun 19, 2020
itsandreramon pushed a commit to itsandreramon/Store that referenced this pull request Feb 26, 2025
…obileNativeFoundation#181)

* Move Fetcher factories into companion

Fetcher factories were global methods, which made them hard
to discover since IDE cannot easily auto-complete.

This PR moves them into the companion of Fetcher while also
making Fetcher a real interface instead of a typealias.

Even though it is a bit more code for the developer, now they
can easily discover how to create a Fetcher by typing Fetcher.

Fixes: MobileNativeFoundation#167

* make rx methods start w/ from too for consistency

* Rename fether factories to be more clear, hopefully :/

* remove fetch method, use invoke instead

* Make Fetcher.from the one that receives a suspend fun.
Create Fetcher.fromFlow for the flowing version.

Rename both SourceOfTruth builder methods to . Rely on param names to disambiguate

* use .of instead, this seems better to me.

We should probably get rid of StoreBuilder.from and make it
Store.builder()

* fix jvm name for SourceOfTruth.of with flow function

* fix RxSourceOfTruth name to match original class

* specify bounds for FactoryFetcher

* updates per PR review

* update graph per SoT rename

* update rxjava3 APIs as well

These appeared after i rebased, missed them completely.
Also fixed some tests, appearantly IJ parameter name refactor does
not always work

* supress wrong unnecessary cast warning

without this, multicaster cannot resolve to the base StoreResponse type

* upgade gradle, try to fix build by disabling caching

* split subscribers

* resubscribe

Co-authored-by: miken <[email protected]>

Co-authored-by: miken <[email protected]>
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.

move Fetcher factory methods into Fetcher

3 participants