CCF icon indicating copy to clipboard operation
CCF copied to clipboard

Remove `network_tables.h`

Open eddyashton opened this issue 3 years ago • 1 comments

We currently define the type and instance of all service tables in a central place, network_tables.h, allowing the frontends to access them as eg tables.member_certs. This is unnecessary - the frontend code only needs to see the type and name of the table, and we don't need a single source exposing the types of all tables (potentially inflating build times).

We could remove network_tables.h and table_names.h, and move each table's name to the per-table header. This makes them individually includable, but still makes the code access points a little uglier (tx.get<MemberCerts>(MEMBER_CERTS) rather than just tx.get(tables.member_certs)). We can simplify that one step further by moving the table name into the static type definition (ie - MemberCerts only has a single name it should be accessed by, so this can be exposed in MemberCerts).

This isn't a high priority, but has been in my head for a while - documenting for posterity.

eddyashton avatar Feb 08 '22 09:02 eddyashton

A related issue is that the table definitions don't actually include the necessary pieces to use those tables, if they rely on some custom serialisers. I don't have a sample at the moment, but for instance if you include service/tables/users.h, but not service/blit.h, then if you actually try to use a handle on a Users table you'll get an obscure compile error saying the BlitSerialiser doesn't know how to handle Pems. This isn't true, it's just defined in a separate header. We should see if we can make this a more instant/localised compile time error,

eddyashton avatar Feb 10 '22 16:02 eddyashton

The original goal is now a non-goal. We found a use for the single central list of all service tables - auto-generating HTTP wrappers for them (#4659). So this issue can be closed.

The second comment is still true, but I didn't find a solution.

eddyashton avatar Mar 09 '23 10:03 eddyashton