[gateway-types] reorganize per RFD 619#9487
Conversation
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
jgallagher
left a comment
There was a problem hiding this comment.
Thanks, I like how this is all shaping up quite a lot. Just some small style notes/preferences.
gateway/src/http_entrypoints.rs
Outdated
| path: Path<PathSp>, | ||
| ) -> Result<HttpResponseOk<SpState>, HttpError> { | ||
| path: Path<latest::component::PathSp>, | ||
| ) -> Result<HttpResponseOk<latest::component::SpState>, HttpError> { |
There was a problem hiding this comment.
This is purely a style choice, but I think I would have imported all of the latest types so that this remained
async fn sp_get(
rqctx: RequestContext<Self::Context>,
path: Path<PathSp>,
) -> Result<HttpResponseOk<SpState>, HttpError> {and maybe would have done the same even in the *-api crate? And then only use explicit (non-latest) versions on the endpoints that need them.
I think this is mostly because adding latest::submodule:: to every type referenced in this file causes a lot of pretty long lines and makes this diff noisier than it would be otherwise. It seems fine to me for an unadorned type to implicitly be "the latest". But I don't feel as strongly about this as I did other things we've already discussed.
There was a problem hiding this comment.
I have mixed opinions here. For new hires, and people not in the code a bunch, the types explicitly being marked latest could be informative at a glance. On the other hand I sympathize pretty heavily with eliminating the repetition and long lines.
I think I prefer John's suggestion, and would just add a comment to the import about unversioned types being latest.
One slight wrinkle is that some type names may conflict on their own. In that case though, the module can still be the prefix or we can use an alias. Nothing really changes how that currently works.
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
No description provided.