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

Remove references to auto-generated in Monitoring README #1801

Closed
bjwatson opened this issue Nov 16, 2016 · 14 comments · Fixed by #2526
Closed

Remove references to auto-generated in Monitoring README #1801

bjwatson opened this issue Nov 16, 2016 · 14 comments · Fixed by #2526
Assignees
Labels
api: monitoring Issues related to the Cloud Monitoring API. priority: p2 Moderately-important priority. Fix may not be included in next release.

Comments

@bjwatson
Copy link

What

Change these lines in the NodeJS Monitoring README. Do not explicitly state that it is an auto-generated library. Also, do not say that "a handwritten layer is not yet available".

Why

GAPIC-only libraries will become the norm as the code generators become capable of producing client libraries faster than we can hand-write them. We do not want to lead with calling them "auto-generated", since that can have negative connotations and is not the defining feature of the library design.

Stating that "a handwritten layer is not yet available" suggests that one might eventually be available. We don't want to lead developers to believe that they might eventually have a different surface to work with (apart from major version updates).

Notes

@callmehiphop I think @landrito originally did this, but I could not assign this ticket to him. Could you or @stephenplusplus give him, @jmuk, and @swcloud the necessary permission to own issues here? Thanks!

FYI @omaray

@bjwatson bjwatson added docs api: monitoring Issues related to the Cloud Monitoring API. labels Nov 16, 2016
@stephenplusplus
Copy link
Contributor

stephenplusplus commented Nov 16, 2016

I wrote those lines, and I think the distinction is very important currently, as there are no docs for the generated API or precedent for an API that doesn't have our conventions; most popularly, the hierarchy between resource types in any given API, i.e. Storage > Bucket > File (storage({ auth }).bucket('bucket-name').file('file-name')) and the ways you can stat/interact with them (object.get(), object.getMetadata(), object.exists(), object.delete(), etc).

Do not explicitly state that it is an auto-generated library.

Why not?

I think a document or addition to the README would be very welcome to explain how we intend to mix autogenerated and handwritten APIs. And if we're not ready for that yet, which I assume we aren't, I think the Monitoring document in its current form is an appropriate way to communicate the state of things now.

Stating that "a handwritten layer is not yet available" suggests that one might eventually be available. We don't want to lead developers to believe that they might eventually have a different surface to work with (apart from major version updates).

I agree on that. I assumed at the time we were going to offer handwritten layers for all generated APIs. Changing this to simply "a handwritten layer is not available for this API." is sufficient.

@bjwatson
Copy link
Author

Mainly because "auto-generated" is not a selling point and isn't the defining feature of the layer. We are currently kicking around better terms to use for the various layers. I'm okay with acknowledging that Monitoring does not have a layer that the other client libraries have, as long as it doesn't sound like it's a sub-optimal experience as a result. I'm interested in @omaray's opinion on this issue.

BTW, if there are idioms that you're using across all/most APIs, we should discuss what they are and whether we can move them into GAPIC/GAX.

@stephenplusplus
Copy link
Contributor

Gotcha, that makes sense. Any names you're considering that you want to share here? Maybe some more eyes on it would help spark some winners. What are the defining features of the generated APIs, from a non-contributor's perspective?

On the idiom note, I'll kick off an issue on the gax repo.

@bjwatson
Copy link
Author

I don't think there's anything secret about it, and I agree that more eyes & brains would generate better results. Let me check with my teammates before exposing their brainstorming to the whole world. :)

Thanks, Stephen.

@stephenplusplus
Copy link
Contributor

Issue sent over in googleapis/gax-nodejs#71

@bjwatson
Copy link
Author

Thanks, Stephen.

@bjwatson bjwatson added priority: p2 Moderately-important priority. Fix may not be included in next release. Type: Enhancement labels Mar 2, 2017
@bjwatson bjwatson assigned landrito and unassigned callmehiphop Mar 2, 2017
@landrito
Copy link
Contributor

landrito commented Aug 9, 2017

The README has been updated to use a template made by @omaray. I made a PR to our staging repo to show the change. @bjwatson does the contents of this readme work for you?

@bjwatson
Copy link
Author

bjwatson commented Aug 9, 2017

@landrito I'm referring to the hand-written README for the google-cloud-node repo.

@bjwatson
Copy link
Author

@stephenplusplus #2526 didn't fix this issue.

Whether or not we want to fix this is up to @omaray and @lukesneeringer. It's a product messaging decision.

@bjwatson bjwatson reopened this Aug 17, 2017
@stephenplusplus
Copy link
Contributor

cc @eoogbe who marked that PR as a fix for this issue (which is why it was auto-closed when the PR merged)

@evaogbe
Copy link
Contributor

evaogbe commented Aug 17, 2017

As far as I can tell, the README no longer has the offending lines, so this should be closed.

@evaogbe
Copy link
Contributor

evaogbe commented Aug 17, 2017

Oh that README. I didn't even notice it was there. Sorry about that.

@stephenplusplus
Copy link
Contributor

The new location has no mentions of auto-generated: https://github.com/googleapis/nodejs-monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: monitoring Issues related to the Cloud Monitoring API. priority: p2 Moderately-important priority. Fix may not be included in next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants