这是indexloc提供的服务,不要输入任何密码
Skip to content

Use atomicfu in paging-common #522

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

Closed
wants to merge 1 commit into from

Conversation

veyndan
Copy link
Contributor

@veyndan veyndan commented Apr 19, 2023

Test: ./gradlew test connectedCheck
Bug: 270612487

Copy link
Member

@claraf3 claraf3 left a comment

Choose a reason for hiding this comment

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

The atomicfu library seems to be in Beta. @ianhanniballake Does this conflict with our general policy to use stable libraries?

@claraf3 claraf3 requested a review from ianhanniballake April 19, 2023 17:57
@dlam
Copy link
Member

dlam commented Apr 19, 2023

We can't use atomicfu in androidx just yet. We've had discussions for this before with compose and datastore. We have a very light dependency on it in nativeMain but it's not stable enough for jvm since we want to bring that stable in a reasonable timeframe.

@veyndan
Copy link
Contributor Author

veyndan commented Apr 19, 2023

Makes sense. I decided to use atomicfu here purely because I saw it was already in your libs.versions.toml file, so I assumed that you're good with using it.

Multiplatform Paging currently doesn't depend on atomicfu, but instead depends on Stately Concurrency. If you look at the source code, on the JVM, it just directly typealiases to java.util.concurrent.atomic.*.

Would you be okay with switching over to Stately Concurrency for this PR instead (which is stable)?

@veyndan
Copy link
Contributor Author

veyndan commented Apr 19, 2023

FYI I've already proposed the usage of Stately in #518

@claraf3
Copy link
Member

claraf3 commented May 9, 2023

Thanks again for your patience on this PR. Im a bit swamped right now but I'll get to this PR as soon as I can. I need to explore other options / write our own locks(?) as we also want to avoid using Stately for this as well.

@claraf3
Copy link
Member

claraf3 commented May 13, 2023

Hi @veyndan just wanted to update you -- we want to hold off on adding new dependencies until we cut Paging 3.2, so this PR will be part of Paging 3.3. This should happen within the month.

@claraf3
Copy link
Member

claraf3 commented Jun 29, 2023

Ended up using stately for Atomics in #577, closing this one as duplicate.

@claraf3 claraf3 closed this Jun 29, 2023
@veyndan veyndan deleted the 270612487/atomic branch June 29, 2023 22:34
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