oxidecomputer / third-party-api-clients

A place for keeping all our generated third party API clients.
https://docs.rs/octorust
MIT License
132 stars 55 forks source link

remove unnecessary feature requirements and fix docs generation #3

Closed r-arias closed 2 years ago

r-arias commented 2 years ago

The generated crates are using #[!feature(async_stream)]. This prevents them from being compiled on stable Rust.

It seems that the feature is not actually used, though (I assume, there was some copy&pasting going on here?), and it can be removed to make the crates compile on stable.

Additionally; the documentation generation had a bug where spaces in the proper_name were not correctly handled, leading to examples in the readme and the docs (see https://crates.io/crates/gsuite-api):

use gsuite_api::Client;

let google admin = Client::new(
    String::from("client-id"),
    String::from("client-secret"),
    String::from("redirect-uri"),
    String::from("token"),
    String::from("refresh-token")
);

and

  • GOOGLE ADMIN_CLIENT_ID
  • GOOGLE ADMIN_CLIENT_SECRET
  • GOOGLE ADMIN_REDIRECT_URI

This pull request adds an identifier_name where spaces are replaced.

Hope this is useful, please let me know if you want me to change anything :slightly_smiling_face:

Susurrus commented 2 years ago

1 references needing to modify the generator, not the code in this project. I ran into the issues addressed by this PR today as well, so I thought I'd dig into this a bit and found that issue. Just wanted to give you a headsup here.

r-arias commented 2 years ago

1 references needing to modify the generator, not the code in this project. I ran into the issues addressed by this PR today as well, so I thought I'd dig into this a bit and found that issue. Just wanted to give you a headsup here.

Hey @Susurrus, thanks for your comment :slightly_smiling_face: I'm not sure I follow, though. This PR does change the generator. All changes to the actual library code result from these generator changes.

Susurrus commented 2 years ago

Sorry, you're correct, I didn't realize the generator changes were included here! My bad.

r-arias commented 2 years ago

No worries, thanks for clarifying!

Shogan commented 2 years ago

Hi, any chance this can get merged and a new crate release published? I would love to use some of this, but am having issues using it on stable rust. Thanks!

r-arias commented 2 years ago

@Shogan I don't know if or when this will be merged, but feel free to clone and add a git dependency to your project.

Make sure you refer to your clone so you don't have to trust me :wink:

Disclaimer: I have not actually used the crate yet; I hope it will be useful for you.

Shogan commented 2 years ago

@Shogan I don't know if or when this will be merged, but feel free to clone and add a git dependency to your project.

Make sure you refer to your clone so you don't have to trust me 😉

Disclaimer: I have not actually used the crate yet; I hope it will be useful for you.

Thanks @r-arias ! I've been a little busy lately, but will hopefully pick this little project I was working on up again soon and will definitely give this a try. Thanks again.

r-arias commented 2 years ago

@Shogan sure :slightly_smiling_face:

@jessfraz disclaimer: sorry for the ping; I don't like to presume on people's time to maintain projects like this one...

However, it's been a while since this PR's been open, people seem to like it (judging by the emoji reactions, at least), and I'd love some feedback on whether it has any chance of getting merged or whether I've gone off in the wrong direction. I'm happy to make some adjustments if you'd like me to. Just tell me what :blush:

Thanks for your work! Cheers, R

jessfraz commented 2 years ago

hey thanks! I recently added some github actions to test, can you rebase and if everything passes will merge, thanks!

r-arias commented 2 years ago

Hi @jessfraz ! Thanks for your response! I've rebased :slightly_smiling_face: the actions don't seem to run automatically, but maybe you can trigger them manually? Either that or I've messed up :sweat_smile:

Cheers, R

martingallagher commented 2 years ago

The async_stream feature has been removed from nightly (https://github.com/rust-lang/rust/pull/93613/files), so it'd be great to get this merged. Currently this will no longer work in stable or nightly tip.

jessfraz commented 2 years ago

okay i pushed this upstream with some other fixes hope its good for you!