JarroVGIT opened a new issue, #1847: URL: https://github.com/apache/datafusion-ballista/issues/1847
This is obviously just a nit, and I am only opening this to reference in a PR if that is desired. Going through the codebase (working on #1829 ), I have noticed that there is an inconsistent usage of the `async_trait` macro, where at some places it is using the re-export of tonic (e.g. `#[tonic::async_trait]`) and in other places it uses the macro from async_trait directly (e.g. `#[async_trait::async_trait]` or a `use` + `#[async_trait]`). ## Using tonic's re-export I guess it makes sense to use tonic's re-export in case of gRPC-related service impls, to signal the relation with gRPC. But I found myself confused by it in two specific places: - `trait ClusterState`: [mod.rs](https://github.com/apache/datafusion-ballista/blob/7be5a3f7668cce29c6a53afb3be155003a3a2273/ballista/scheduler/src/cluster/mod.rs#L154-L158) - `trait Job`: [mod.rs](https://github.com/apache/datafusion-ballista/blob/7be5a3f7668cce29c6a53afb3be155003a3a2273/ballista/scheduler/src/cluster/mod.rs#L277-L281) The impl's of both traits are **not** using tonic's re-export, so I propose to change those two instances to use the `async_trait` directly. ## Using `#[async_trait::async_trait]` or a `use` + `#[async_trait]`) The project uses the fully qualified version 7 times and the `use` + unqualified (do I say that correctly?) version 14 times. Not really a preference but using the fully qualified version will shorten the `use` blocks and makes it visually consistent. Is it okay to put a PR in for this? Or rather not? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
