Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

Remove Messenger actor and use withContext instead #31

Merged
merged 19 commits into from
Apr 10, 2020

Conversation

twyatt
Copy link
Member

@twyatt twyatt commented Mar 3, 2019

Inspired by comment by elizarov (on Jun 15) in Kotlin/kotlinx.coroutines#87:

when you ask an [sic] actor and want a result back the proper design would be to have a suspend fun with a normal (non-deferred) Result. However, please note that this whole ask & wait pattern is an anti-pattern in actor-based systems, since it limits scalability.

Additionally, changing over to using withContext made the library much easier to write tests for.

Took the refactor opportunity to:

  • Update the Kotlin Coroutines library and integrate Flow for data streams
  • Drop experimental from package name (since we're no longer using experimental Coroutines)
  • Update copyright headers
  • Write/utilize JUnit test rule that only outputs debug logs on test failure
  • Add Kotlinter
  • Write lots of unit tests

@twyatt twyatt added this to the 0.8 milestone Jun 20, 2019
@twyatt twyatt force-pushed the travis/shoot-the-messenger branch from edef027 to 36f763a Compare June 26, 2019 05:51
@twyatt twyatt force-pushed the travis/shoot-the-messenger branch 2 times, most recently from d1e2203 to 635f9b0 Compare April 5, 2020 03:19
@codecov
Copy link

codecov bot commented Apr 5, 2020

Codecov Report

Merging #31 into develop will increase coverage by 54.00%.
The diff coverage is 74.48%.

Impacted file tree graph

@@              Coverage Diff               @@
##             develop      #31       +/-   ##
==============================================
+ Coverage      20.37%   74.38%   +54.00%     
- Complexity        17       35       +18     
==============================================
  Files             14       14               
  Lines            476      242      -234     
  Branches          42       28       -14     
==============================================
+ Hits              97      180       +83     
+ Misses           368       52      -316     
+ Partials          11       10        -1     
Impacted Files Coverage Δ Complexity Δ
core/src/main/java/android/BluetoothDevice.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
core/src/main/java/logger/Logger.kt 0.00% <0.00%> (ø) 0.00 <0.00> (?)
processor/src/main/java/GattProcessor.kt 100.00% <ø> (ø) 4.00 <0.00> (?)
timber-logger/src/main/java/AndroidTagGenerator.kt 71.42% <ø> (ø) 3.00 <0.00> (ø)
timber-logger/src/main/java/TimberLogger.kt 0.00% <ø> (ø) 0.00 <0.00> (ø)
core/src/main/java/logger/AndroidLogger.kt 33.33% <33.33%> (ø) 1.00 <1.00> (?)
core/src/main/java/gatt/CoroutinesGatt.kt 51.42% <51.42%> (ø) 8.00 <8.00> (?)
throw/src/main/java/GattOrThrow.kt 83.33% <71.42%> (ø) 0.00 <0.00> (?)
core/src/main/java/gatt/Events.kt 75.40% <75.40%> (ø) 0.00 <0.00> (?)
core/src/main/java/gatt/GattCallback.kt 83.33% <83.33%> (ø) 16.00 <16.00> (?)
... and 14 more

@twyatt twyatt force-pushed the travis/shoot-the-messenger branch 15 times, most recently from 7b04eea to 8bfe1c0 Compare April 7, 2020 20:36
Inspired by [comment] by elizarov (on Jun 15) in
Kotlin/kotlinx.coroutines#87:

> when you ask and actor and want a result back the proper design would
> be to have a `suspend fun` with a normal (non-deferred) `Result`.
> However, please note that this whole ask & wait pattern is an
> anti-pattern in actor-based systems, since it limits scalability.

[comment]: Kotlin/kotlinx.coroutines#87 (comment)
twyatt added 5 commits April 8, 2020 11:49
Just want to make sure close wasn't already closed, as that would make
the close call check later useless.
Copy link

@Chris-Corea Chris-Corea left a comment

Choose a reason for hiding this comment

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

Great job, dude. Holy moly.

core/src/main/java/gatt/CoroutinesGatt.kt Outdated Show resolved Hide resolved
Copy link

@davidtaylor-juul davidtaylor-juul left a comment

Choose a reason for hiding this comment

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

🚀 and tests 💯

@twyatt twyatt merged commit 95f5115 into develop Apr 10, 2020
@twyatt twyatt deleted the travis/shoot-the-messenger branch April 10, 2020 07:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants