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

Improve the API reference #4133

Open
globsterg opened this issue May 26, 2024 · 4 comments
Open

Improve the API reference #4133

globsterg opened this issue May 26, 2024 · 4 comments
Labels
docs KDoc and API reference

Comments

@globsterg
Copy link
Contributor

Describe the bug

kotlinx-datetime was given new and updated documentation... kotlinx-corotoutines could also benefit from the same treatment.

Provide a Reproducer

Going through the list from kotlinx-datetime (Kotlin/kotlinx-datetime#347):

  • Extensively document what the entity represents, whether it follows ISO-8601, how the entity can be acquired, and what time scale it comes from. Example
  • Provide a minimum of how the entity can be used. Example
  • Thoroughly explain the allowed and expected contract and behaviours and system-specifics. Example
  • If applicable, provide contexts in which the entity is expected/recommended to be used (e.g. our README shines at that) and how it interacts with other (core) entities. Example
  • Provide a basic usage example. Example
  • If applicable, provide details about conversion rules, gotchas and recommendations. Example (it's not mentioned what happens when cancellation goes the other way)
  • Where appropriate, information about conversions and interactions with native types (e.g. Instant <-> Java Instant CoroutineDispatcher <-> Java Executor conversion)
  • Ensure that information used in our README is, in fact, deducible from the core entities documentation
@globsterg globsterg added the bug label May 26, 2024
@globsterg
Copy link
Contributor Author

If you allow it, I would like to contribute to this myself. I am implementing my own programming language and need to learn more about the real-world asynchronous runtimes before deciding on what the concurrency story will look like, and a deep dive into an existing framework would help me tremendously.

Basically, it's a trade offer.

I receive:

  • Your corrections to my understanding of how your library works.
  • Sense of fulfillment, having helped the plethora of users of an important library.

You receive:

  • Work that fills some gaps in your documentation here and there.
  • Sense of fulfillment, having spread your knowledge to a new programming ecosystem that is hopefully also going to be useful (and possibly to Kotlin, as well!).

The effort I can extend on this is limited to a about one workweek this June. My contribution can be as localized or as extensive as you'd like, given the time constraints. I can also start with some small changes before you decide if you are interested in my output.

@qwwdfsad qwwdfsad added docs KDoc and API reference and removed bug labels May 27, 2024
@qwwdfsad
Copy link
Collaborator

qwwdfsad commented May 27, 2024

Hey, thanks for filing this!

We were planning to jump on it ourselves (as there is a lot of work to revise all the API entry points), and it's easy to split the work -- esp. taking into account that we are trying to align the documentation approach across the libraries, and thus initially there will be a lot of ping-pong discussions.

We would appreciate your help!
We have a constraint, though -- starting from the end of the week, both maintainers will be on vacation, and there will be ~2.5 weeks of radio-silence from us. For more straightforward and lower-levelish APIs (e.g. intrinsic, start coroutine, channels) maybe @fzhinkin might chime in the meantime.

If it works for you, I propose the following:

  • Pick some reasonably small API surface (e.g., not all flow operators) that is to your liking and open a PR
  • We'll start converging on our way towards API documentation, and the next PRs are likely to be much smoother
  • If you want to take some surface to document, please drop a line here so we can avoid doing the same thing
  • If at any point you'll decide that it's enough -- no worries. Feel free to open a draft PR, let us know, and we'll handle it from there

For inspiration, I suggest looking at Kotlin/kotlinx-datetime#367, and I also would ask you to avoid the @sample tag in coroutines for now -- everything is samplified via inline code blocks.

Even if this is not working, I still appreciate the way you handled and proposed it, thanks!

@globsterg
Copy link
Contributor Author

Thank you for your response, being open to collaboration, and describing the circumstances in detail.

I've looked through Kotlin/kotlinx-datetime#367, noting both how the documentation proposed in the PR is structured and what questions were raised during the review; I've also browsed through https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/ and familiarized myself with the API entries you have. I will try my best to follow the established form.

The plan you propose does look good! Let's double-check the specifics, then:

  • During your vacations, I will make a PR with improvements to the KDoc-based documentation of low-level things and add @fzhinkin as a reviewer.
  • While the review is in progress, I work with other API entries. Because all maintainers are on vacation, I don't need to notify anyone about what I'm doing, unless someone else decides to join us in writing the documentation.
  • When the review is finished, I will update the rest of my work in accordance with the points highlighted there and make additional PRs.
  • When the maintainers return, we go through additional rounds of review. If I still have the resources to work on this, we discuss how to split further work and proceed from there.

Does this sound about right?

In the meantime, I've opened a PR in my own fork (https://github.com/globsterg/kotlinx.coroutines/pull/1/files) where I will keep a mishmash of improvements of small things that I notice as I go.

@globsterg
Copy link
Contributor Author

Hi! I think I will do this:

If at any point you'll decide that it's enough -- no worries. Feel free to open a draft PR, let us know, and we'll handle it from there

Even though I only touched some parts of the framework, I had to dive deep and learned a lot from it. The most impressive insight for me was the elegant solution to the "async drop" problem: there is always a coroutine dispatcher responsible for each task, and that dispatcher guarantees that the task will run to completion in every scenario, which means, together with the Java-style exception-based error handling and "drop" implementations being put in finally blocks, that every resource will be released by someone eventually.

Thank you for letting me dive deep into your framework and the clear corrections along the way. I hope my contributions will bring you some utility. For now, though, my async research time budget is depleted, and as the perfect is the enemy of the good, I'd like to move on to other problems. However, if you want me to, I am ready to implement those corrections to my PRs that are mostly mechanical and don't require deeper dives into the code on my part, as I think it would be rude to just leave you with a pile of text that you maybe do not like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs KDoc and API reference
Projects
None yet
Development

No branches or pull requests

2 participants