Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3)

Issue 11700043: code review 11700043: present/dir.go: be more idiomatic when documenting bool... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 6 months ago by r
Modified:
7 years, 6 months ago
Reviewers:
dsymonds
CC:
golang-dev, dsymonds
Visibility:
Public.

Description

present/dir.go: be more idiomatic when documenting boolean return values. Phrases like "returns whether or not the image is opaque" could be describing what the function does (it always returns, regardless of the opacity) or what it returns (a boolean indicating the opacity). Even when the "or not" is missing, the phrasing is bizarre. Go with "reports whether", which is still clunky but at least makes it clear we're talking about the return value.

Patch Set 1 #

Patch Set 2 : diff -r afcde7da5687 https://code.google.com/p/go.talks #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
present/dir.go View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 3
r
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go.talks
7 years, 6 months ago (2013-07-23 00:28:45 UTC) #1
dsymonds
LGTM
7 years, 6 months ago (2013-07-23 00:34:35 UTC) #2
r
7 years, 6 months ago (2013-07-23 00:37:18 UTC) #3
*** Submitted as
https://code.google.com/p/go/source/detail?r=0f2497c44599&repo=talks ***

present/dir.go: be more idiomatic when documenting boolean return values.
Phrases like "returns whether or not the image is opaque" could be
describing what the function does (it always returns, regardless of
the opacity) or what it returns (a boolean indicating the opacity).
Even when the "or not" is missing, the phrasing is bizarre.

Go with "reports whether", which is still clunky but at least makes
it clear we're talking about the return value.

R=golang-dev, dsymonds
CC=golang-dev
https://codereview.appspot.com/11700043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b