write description of subsystem initialization/shutdown architecture

changed milestone to %Tor: 0.4.3.x-final

Trac:
Parent Ticket: #29215 (moved)

added actualpoints::.2 component::core tor/tor milestone::Tor: 0.4.3.x-final network-team-roadmap-september owner::nickm parent::29215 priority::medium resolution::implemented reviewer::catalyst s31-docs severity::normal sponsor::31-must status::closed type::task labels

Trac:
Owner: N/A to nickm
Status: new to assigned

these are tasks not defects

Trac:
Type: defect to task

See branch ticket32211 for a small first draft here. I imagine that more documentation is probably desirable; any guidance would be welcome. PR at https://github.com/torproject/tor/pull/1520

Trac:
Actualpoints: N/A to .2
Status: assigned to needs_review

Trac:
Reviewer: N/A to catalyst

Replying to nickm:

See branch ticket32211 for a small first draft here. I imagine that more documentation is probably desirable; any guidance would be welcome. PR at https://github.com/torproject/tor/pull/1520 Thanks! Looks good so far. What areas do you think need more work?

We might want to explain why we would want to add new fields in the middle of subsys_fns_t.

In initialization.dox, maybe move the reference to subsys_fns_t closer to the beginning of that paragraph? Maybe like

subsys_fns_t describes a subsystem and a set of functions [...] To define a subsystem...

As a minor style nit, do we need to use backslashes for Doxygen commands in that places you use them here? I would like us to move towards more consistently using at-signs for Doxygen commands.

Replying to catalyst:

Replying to nickm:

See branch ticket32211 for a small first draft here. I imagine that more documentation is probably desirable; any guidance would be welcome. PR at https://github.com/torproject/tor/pull/1520 Thanks! Looks good so far. What areas do you think need more work?

I don't know, TBH. I'm hoping that there will be questions I can answer more about.

We might want to explain why we would want to add new fields in the middle of subsys_fns_t.

I don't understand -- as opposed to at the end of it? Or why we'd add new fields at all?

In initialization.dox, maybe move the reference to subsys_fns_t closer to the beginning of that paragraph? Maybe like

subsys_fns_t describes a subsystem and a set of functions [...] To define a subsystem...

Sounds good.

As a minor style nit, do we need to use backslashes for Doxygen commands in that places you use them here? I would like us to move towards more consistently using at-signs for Doxygen commands.

I'm okay trying to remember to use @ signs in the future, but I'd rather not adopt a rule forcing them without tooling to convert and enforce. Does that sound okay?

Replying to nickm:

Replying to catalyst:

We might want to explain why we would want to add new fields in the middle of subsys_fns_t.

I don't understand -- as opposed to at the end of it? Or why we'd add new fields at all? Yes, as opposed to at the end. Hm, given that this is justifying asking people to use named initializers, maybe we should also say that they help with code readability when many pointers need to be null? Also, I'm guessing that sometimes there are reasons to keep a logical grouping of fields together? (I'm pretty sure we're not optimizing for storage layout here given the existing order of fields in the structure.)

As a minor style nit, do we need to use backslashes for Doxygen commands in that places you use them here? I would like us to move towards more consistently using at-signs for Doxygen commands.

I'm okay trying to remember to use @ signs in the future, but I'd rather not adopt a rule forcing them without tooling to convert and enforce. Does that sound okay? Yes. I'm happy if we adopt a guideline (not requirement) to prefer at-signs for Doxygen commands in new Doxygen comments.

Okay. I've updated and merged this branch. Thanks!

Trac:
Resolution: N/A to implemented
Status: needs_review to closed

closed

added 1h 36m of time spent

mentioned in issue #29215 (moved)

moved to tpo/core/tor#32211 (closed)