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.

Sign up for GitHub

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

Jump to bottom

vision: update to v1.1 #2041

Merged
merged 3 commits into from
Mar 3, 2017
Merged

vision: update to v1.1 #2041

merged 3 commits into from
Mar 3, 2017

Conversation

stephenplusplus
Copy link
Contributor

@stephenplusplus stephenplusplus commented Mar 2, 2017 •
edited
Loading

No description provided.

@stephenplusplus stephenplusplus added the api: vision Issues related to the Cloud Vision API. label Mar 2, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 2, 2017
@callmehiphop
Copy link
Contributor

@stephenplusplus looks like there's an issue with collecting coverage on CI.

@stephenplusplus
Copy link
Contributor Author

Yeah, been working on that. Started a re-build, hopefully it goes this time.

callmehiphop
callmehiphop reviewed Mar 2, 2017
View reviewed changes
Copy link
Contributor

@callmehiphop callmehiphop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in general we could use better descriptions for all the new methods, they seem lacking to the point where I could gather that information from the method name alone.

packages/vision/src/index.js Outdated
// jscs:enable maximumLineLength

/**
* Detect the crops within an image.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

packages/vision/src/index.js
*
* @private
*/
Vision.formatCropHintsAnnotation_ = function(cropHintsAnnotation, options) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

packages/vision/src/index.js
*
* @private
*/
Vision.formatFullTextAnnotation_ = function(fullTextAnnotation, options) {

This comment was marked as spam.

Sign in to view

@stephenplusplus
Copy link
Contributor Author

PTAL!

Regarding the simplistic doc messages, we normally use these (these are in fact copied from other methods, for example, detectFaces(): "Run face detection against an image". I'm sure these could be improved if we wanted to elaborate on possible use cases, etc. Currently, we only go above and beyond a long-hand form of the method name where there are technical considerations or non-obvious GCN options (i.e. not non-obvious API options). Otherwise, we link upstream and let the official doc and tech writers do the explaining. If you want to explore going deeper with our own method docs, please open a new ticket for discussion.

@stephenplusplus stephenplusplus merged commit 89fa199 into googleapis:master Mar 3, 2017
@jmuk
Copy link
Contributor

jmuk commented Mar 3, 2017

Thank you for merging!

@stephenplusplus
Copy link
Contributor Author

👍 will be doing a bunch of releases shortly.

@sofisl sofisl mentioned this pull request Jan 17, 2023
Merged
4 tasks
sofisl pushed a commit that referenced this pull request Jan 17, 2023
@stephenplusplus
vision: update to v1.1 (#2041)
3694afe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@callmehiphop callmehiphop callmehiphop left review comments

Assignees
No one assigned
Labels
api: vision Issues related to the Cloud Vision API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stephenplusplus @callmehiphop @jmuk @googlebot