Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade apis to v2 & allow to select apis via feature flag for optimized compile times #565

Merged
merged 4 commits into from
Aug 26, 2024

Conversation

sprudel
Copy link
Contributor

@sprudel sprudel commented Aug 24, 2024

This is achieved by the following steps:

  • generate clients including feature flag
  • guard individual apis with features flags
  • switch from v2beta to v2 apis

@sprudel
Copy link
Contributor Author

sprudel commented Aug 26, 2024

@buehler getting this PR reviewed fast would be highly appreciated. Happy to discuss any changes/adaption needed :-)

buehler
buehler previously approved these changes Aug 26, 2024
@buehler
Copy link
Collaborator

buehler commented Aug 26, 2024

Hi @sprudel

Thanks for your contribution!
I really like the changes, especially since everybody is able to selectively use APIs. What I don't understand is the protocc_insertion_deletion_point for features in the cargo.toml file. Can you elaborate what happens there?

Also: Testing/Linting seems to be angry with some changes. Otherwise LGTM.

@sprudel
Copy link
Contributor Author

sprudel commented Aug 26, 2024

Hi,
thanks a lot for the quick feedback.

https://docs.rs/protoc-gen-prost-crate/0.4.1/protoc_gen_prost_crate/#cargo-manifest-template describes the mechanism.
Basically gen_crate allows to generate the feature flags which need to be added to the Cargo.toml. During generation it must be known where to put the features. This is the reason for these special comments.

I have considered moving the generation to a separate crate, but this would probably make the publishing process more complex.

@sprudel
Copy link
Contributor Author

sprudel commented Aug 26, 2024

@buehler I fixed the clippy warnings, should be good now.
I plan to tackle the comments of the other mr this evening.

@buehler buehler enabled auto-merge (squash) August 26, 2024 12:18
@buehler buehler merged commit d1528a2 into smartive:main Aug 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants