Skip to content

Conversation

@FranAguilera
Copy link
Contributor

@FranAguilera FranAguilera commented Jun 30, 2025

What

Removes dependency on com.github.michaelbull for 'CaptureResult'

NOTE: These updated CaptureResult will be used on #474

Verification

Verified on this session

@FranAguilera FranAguilera marked this pull request as draft June 30, 2025 15:22
@bitdriftlabs bitdriftlabs deleted a comment from github-actions bot Jul 1, 2025
@FranAguilera FranAguilera changed the title [WIP] [Android] Remove com.github.michaelbull [Android] Remove com.github.michaelbull Jul 1, 2025
* A monad for modeling success or failure operations in the Capture SDK.
*/
typealias CaptureResult<V> = com.github.michaelbull.result.Result<V, Error>
sealed class CaptureResult<out V> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since refactoring this will touch all existing references, why CaptureResult instead of just Result since this is already in the capture package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually i thought about it, but wanted to minimize noise to existing customer that rely on CaptureResult

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, I didn't realize this class/alias was public

@FranAguilera FranAguilera marked this pull request as ready for review July 1, 2025 10:40
* A monad for modeling success or failure operations in the Capture SDK.
*/
typealias CaptureResult<V> = com.github.michaelbull.result.Result<V, Error>
sealed class CaptureResult<out V> {
Copy link
Contributor

Choose a reason for hiding this comment

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

you should take a look at the library we used, it can be helpful as a reference implementation for things like this:

The Result type is modelled as an inline value class. This achieves zero object allocations on the happy path.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not saying we have to do it that way but it's a neat property of the library

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup very neat indeed

@FranAguilera FranAguilera requested review from kattrali and murki July 1, 2025 12:54
@github-actions
Copy link

github-actions bot commented Jul 1, 2025

📦 APK Size Report

Metric Size (KB)
Baseline 3610
Current 3606
Difference -4

✅ APK size decreased by 4 KB.

@bitdriftlabs bitdriftlabs deleted a comment from github-actions bot Jul 1, 2025
val typedRequest = DeviceCodeRequest(deviceId)

apiClient.perform<DeviceCodeRequest, DeviceCodeResponse>(HttpApiEndpoint.GetTemporaryDeviceCode, typedRequest) { result ->
completion(result.map { it.code })
Copy link
Contributor

Choose a reason for hiding this comment

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

ok so it looks like all things being equal this is the only nice method we lost (map), which doesn't sound like the end of the world to me

Copy link
Contributor

@murki murki left a comment

Choose a reason for hiding this comment

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

I'm overall supportive of this change.

@FranAguilera FranAguilera enabled auto-merge (squash) July 1, 2025 15:25
@github-actions
Copy link

github-actions bot commented Jul 1, 2025

📦 APK Size Report

Metric Size (KB)
Baseline 3610
Current 3606
Difference -4

✅ APK size decreased by 4 KB.

@FranAguilera FranAguilera merged commit 32d11c9 into main Jul 1, 2025
14 checks passed
@FranAguilera FranAguilera deleted the franjam/remove-kotlin-result branch July 1, 2025 15:47
@github-actions github-actions bot locked and limited conversation to collaborators Jul 1, 2025
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.

4 participants