Byron / google-apis-rs

A binding and CLI generator for all Google APIs
http://byron.github.io/google-apis-rs
Other
983 stars 132 forks source link

Revert "Merge branch 'format_types'" #463

Closed andrewbaxter closed 6 months ago

andrewbaxter commented 6 months ago

This reverts commit ed5dab2dbd1aa8d126b1253e8020bcf964f0ee34, reversing changes made to 93c8601fdca62f92f164957d9e86220f4f1b10f7.

There are some manual changes to include reverts of subsequent mrs, plus some manual fixups. It builds, but I don't know if it's correct yet.

Related to discussion in https://github.com/Byron/google-apis-rs/pull/446

andrewbaxter commented 6 months ago

Okay AFAICT this works (no compile errors for storage or kms at least). I'm not sure if I should regenerate everything in this MR or as a separate MR afterwards or what.

Byron commented 6 months ago

Thanks a lot for going through with it!

I think this tests expectation could be fixed, the failure appears genuine. Could you do that?

Thanks again for giving it another shot.

I'm not sure if I should regenerate everything in this MR or as a separate MR afterwards or what.

Please do not regenerate anything, such contributions are impossible to review. I regenerate myself before making a release.

andrewbaxter commented 6 months ago

I think this tests expectation could be fixed, the failure appears genuine. Could you do that?

Ah yep. I think I pushed a fix, but I'll take another look tomorrow if there's still issues.

Please do not regenerate anything, such contributions are impossible to review. I regenerate myself before making a release.

Okay perfect, thanks!

andrewbaxter commented 6 months ago

Sorry, I'll have to look at that failure tomorrow.

Byron commented 6 months ago

No worries, this failure is expected, some python breakage that just happens without any change.

I think this PR will be merged soon.

andrewbaxter commented 6 months ago

Oh, per the comment #446 it sounds like going with standard base64 might be the way to go? Or are you still thinking that's risky?

Byron commented 6 months ago

After having caught up, and given your experience by now with the codebase, I think you could try to keep what is there locally, change the encoding to base64 and that should solve all problems, right?

If not, we can still merge this PR and probably leave the encoding/decoding to the user.

I will wait until I hear from you.

andrewbaxter commented 6 months ago

Closed in favor of https://github.com/Byron/google-apis-rs/pull/446