Byron / google-apis-rs

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

Make field mask inner public #465

Closed andrewbaxter closed 9 months ago

andrewbaxter commented 9 months ago

The inner value was private so it couldn't be constructed directly (without deserializing from a string indirectly). Creating it directly seems safer than deserializing, especially since I'm unfamiliar with the format.

Was it intended for this to be constructed directly or is there another mechanism for setting these up? I thought the api might do it directly but it doesn't atm afaict.

andrewbaxter commented 9 months ago

Okay sounds good. I made the change, and tested it locally although I was already using a non-snake-case string "state".

That said, as you mentioned the specification isn't exactly simple. I think in order to use the field correctly in the first place users will have to read up on its use, and if despite doing that they're passing the wrong values in it might be best to allow the api call to fail to let them know they need to re-read. Magically lowercasing but not checking field existence, etc might be more complex implicit behavior.

andrewbaxter commented 9 months ago

Or were you thinking it should just be FieldMask(pub String)? I wouldn't mind that, it'd convey to users that they're completely on their own which I think might be good.