Skip to content
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

NF: document that withCol and withQueue executes in IO #13950

Merged
merged 2 commits into from Jun 9, 2023

Conversation

Arthur-Milchior
Copy link
Member

I had some doubt about which function moves to IO dispatcher, whether it was launch method or withCol. I expect this documentation may save a minute to other devs too.

@@ -58,8 +58,8 @@ object CollectionManager {
var emulateOpenFailure = false

/**
* Execute the provided block on a serial queue, to ensure concurrent access
* does not happen.
* Execute the provided block in the IO dispatchers on a serial queue.
Copy link
Contributor

@dae dae Jun 4, 2023

Choose a reason for hiding this comment

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

Suggested change
* Execute the provided block in the IO dispatchers on a serial queue.
* Execute the provided block on a serial background queue, to ensure concurrent access
does not happen.
* The background queue is run in a Dispatchers.IO context.

Copy link
Member

Choose a reason for hiding this comment

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

Involving Dispatchers.IO in brackets will make it clickable, i.e. [Dispatchers.IO]

@@ -88,6 +88,8 @@ object CollectionManager {
/**
* Execute the provided block with the collection, opening if necessary.
*
* Execution is done in the IO dispatchers.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Execution is done in the IO dispatchers.
* Calls are serialized, and run in background Dispatchers.IO thread.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry Arthur, I had a typo here: it should have read "run in a". Might as well add another [] at the same time.

Copy link
Contributor

@dae dae left a comment

Choose a reason for hiding this comment

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

Just some minor wording bikeshedding.

@BrayanDSO BrayanDSO added the Needs Author Reply Waiting for a reply from the original author label Jun 5, 2023
I had some doubt about which function moves to IO dispatcher, whether it was
launch method or withCol. I expect this documentation may save a minute to other
devs too.
@Arthur-Milchior
Copy link
Member Author

I'm all for bikeshedding here, given that my goal was to improve documentation of a part of the code that seems to be non intuitive. (Or at least, was not to me at first)

@Arthur-Milchior Arthur-Milchior removed the Needs Author Reply Waiting for a reply from the original author label Jun 6, 2023
@BrayanDSO BrayanDSO added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) squash-merge The pull request currently requires maintainers to "Squash Merge" labels Jun 9, 2023
@BrayanDSO BrayanDSO merged commit fcb4e25 into ankidroid:main Jun 9, 2023
8 of 9 checks passed
@github-actions github-actions bot added this to the 2.16 release milestone Jun 9, 2023
@github-actions github-actions bot removed Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) squash-merge The pull request currently requires maintainers to "Squash Merge" labels Jun 9, 2023
@Arthur-Milchior Arthur-Milchior deleted the documentIO branch June 10, 2023 10:48
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.

None yet

3 participants