I am slightly concerned direct references to these methods could get out of date. Can't we just write these down in env.h and link to the headers instead?

Can't we just write these down in env.h and link to the headers instead?

I’m not sure what you mean? But either way, I think keeping this doc up to date and making sure of that during reviews is something we’ll have to do either way.

I think keeping this doc up to date and making sure of that during reviews is something we’ll have to do either way.

I think if keeping this doc up-to-date is going to be a blocker for landing changes in src/, we should just move the information about the internal classes into the headers as comments, and link to those in the README. For one if the README is too long it becomes difficult to read, also it's going to be a pain to remember updating this doc while changing the code - but slightly less so if the comments live in the headers.

Also it feels weird to me that in order to understand some of these classes at a higher level, you either have to read the code, or read this doc because what's supposed to be said in comments are moved into a markdown file...I think it would be more helpful if the README is just a table of contents.

tbh, I really like the proposed format. It has a flow, the necessary level of abstraction, example code at right places, and caveats and cautions as appropriate - something either not possible in code comments, or not easily consumable. IMO inline comments are rich with the context, but weak in connecting bigger views, and more importantly, discoverability.

If you have items that you feel should also be documented more explicitly in a header file, I’m happy to do that, but generally I agree with @gireeshpunathil – this is not supposed to be an exhaustive guide or reference documentation, but rather paint the big picture and distinguish what’s important from what’s not.

and more importantly, discoverability.

I feel the opposite - I think content in the README is more difficult to discover than content in the headers, especially when the README becomes long. For me it's rare that I'll read a doc, top to bottom, to understand a code base and remember all these identifier names. I usually run into these names when I hack on the code and I expect to see explanations in the headers.

this is not supposed to be an exhaustive guide or reference documentation, but rather paint the big picture and distinguish what’s important from what’s not.

That's what I think this document should be like as well, but with details like

They can be added and removed through by using env->AddCleanupHook(callback, hint); and env->RemoveCleanupHook(callback, hint);, where callback takes a void* hint argument.

and

If a libuv request is not managed through a [ReqWrap][] instance, the env->IncreaseWaitingRequestCounter() and env->DecreaseWaitingRequestCounter() functions need to be used to keep track of the number of active libuv requests.

It does not seem to be the case. These references to specific method names and their arguments do not seem to be necessary in terms of giving the reader a bigger view of the code base.

and more importantly, discoverability.

I feel the opposite - I think content in the README is more difficult to discover than content in the headers, especially when the README becomes long.

That’s somewhat solvable by adding a table of contents, if you like?

this is not supposed to be an exhaustive guide or reference documentation, but rather paint the big picture and distinguish what’s important from what’s not.

That's what I think this document should be like as well, but with details like

They can be added and removed through by using env->AddCleanupHook(callback, hint); and env->RemoveCleanupHook(callback, hint);, where callback takes a void* hint argument.

and

If a libuv request is not managed through a [ReqWrap][] instance, the env->IncreaseWaitingRequestCounter() and env->DecreaseWaitingRequestCounter() functions need to be used to keep track of the number of active libuv requests.

It does not seem to be the case. These references to specific method names and their arguments do not seem to be necessary in terms of giving the reader a bigger view of the code base.

Well … where would you document these, especially the latter one? It’s easy to add inline docs to ReqWrap but what do you do when you want to describe when it’s about not using ReqWrap?

In any case, I don’t feel like these give away too much detail. Judging from the current contents of the file in this PR, the number of yearly PRs that would require updating it is probably in the single digits.

That’s somewhat solvable by adding a table of contents, if you like?

I think that does not help in this case. To quote my edit:

For me it's rare that I'll read a doc, top to bottom, to understand a code base and remember all these identifier names. I usually run into these names when I hack on the code and I expect to see explanations in the headers.

Now this is solvable if in the headers we point the readers to the README for more information, but that feels kind of weird.

where would you document these, especially the latter one?

I think if the goal is to give a big picture in the README, for the latter one it can just say something like:

If a libuv request is not managed through a [ReqWrap][] instance, a counter is used to make sure it gets cleaned up.

And the method names can be documented in the req_wrap.h or env.h. The README can just point the readers to that header to see these details. This is a README for src/ after all, so I'd expect it to be more of a table of contents for all these files under src/.

Sorry to be late. This link seems 404 now.

Is there a typo here?