Closed future-highway closed 1 year ago
The command and implementation are now being generated for protos that the macro can handle. We can't handle code generation for the CRI services yet.
Updated the macro to have 2 paths.
subcommand
is for use in cases where the macro produces valid code (right now that includes only allowing 1 value for repeated fields). If the macro thinks it can't generate the right code, it will explicitly panic or hopefully just fail to produce code that compiles.subcommand_for_dev_only
will try to generate code regardless of if it compiles (it will try to not panic). The use for this is to generate as much as possible, expand the macro, and then fix the code.You can see examples of the amount of code generated by looking at the commented out code for the cri services.
This PR builds on #287.
The intention is for the macro to replace the command enum and implementation, but that is proving challenging (at least for me), specifically when mapping from the command variants to the requests. So, the current state only replaces the enum, which is not much gain at all. I'd argue we would be worse off using the macro if this is all that is achieved.
Putting a bit more in the macro is possible (manually write the mapping using the
From
trait, which the macro would call, instead of manually writing the impl), but I consider it another step backwards. For context aFrom
implementation for theCellServiceCommands::Allocate
variant would be:Though I think we are worse off needing to implement the
From
traits, I'm going to push an additional commit that gets the PR to that point, because it is technically closer to the goal.An important additional note: the macro doesn't cover all possible proto types (e.g. enums, oneof), and generally feels fragile.
Overall, unless I or someone else can get the macro to the goal (at which point we can work on making it less fragile), I think we shouldn't use the macro.