Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(170)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 5544073: code review 5544073: image/color: simplify documentation

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 12 months ago by crawshaw1
Modified:
13 years, 12 months ago
Reviewers:
nigeltao
CC:
nigeltao, dsymonds, adg, golang-dev
Visibility:
Public.
image/color: simplify documentation

Patch Set 1 #

Patch Set 2 : diff -r f690897afe60 https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r f690897afe60 https://go.googlecode.com/hg/ #

Total comments: 2

Patch Set 4 : diff -r f690897afe60 https://go.googlecode.com/hg/ #

Created: 13 years, 12 months ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -37 lines) Patch
M src/pkg/image/color/color.go View 1 2 3 4 chunks +32 lines, -31 lines 0 comments Download
M src/pkg/image/color/ycbcr.go View 1 3 chunks +6 lines, -6 lines 0 comments Download
Total messages: 12
|
crawshaw1
Hello nigeltao@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
13 years, 12 months ago (2012年01月15日 23:34:48 UTC) #1
Hello nigeltao@golang.org (cc: golang-dev@googlegroups.com),
I'd like you to review this change to
https://go.googlecode.com/hg/ 
Sign in to reply to this message.
dsymonds
Wouldn't it be better to fix godoc instead?
13 years, 12 months ago (2012年01月15日 23:36:13 UTC) #2
Wouldn't it be better to fix godoc instead?
Sign in to reply to this message.
crawshaw1
On Mon, Jan 16, 2012 at 10:36 AM, David Symonds <dsymonds@golang.org> wrote: > Wouldn't it ...
13 years, 12 months ago (2012年01月15日 23:43:12 UTC) #3
On Mon, Jan 16, 2012 at 10:36 AM, David Symonds <dsymonds@golang.org> wrote:
> Wouldn't it be better to fix godoc instead?
I cannot decide if godoc is broken.
Sign in to reply to this message.
dsymonds
On Mon, Jan 16, 2012 at 10:43 AM, David Crawshaw <crawshaw@google.com> wrote: > On Mon, ...
13 years, 12 months ago (2012年01月15日 23:48:07 UTC) #4
On Mon, Jan 16, 2012 at 10:43 AM, David Crawshaw <crawshaw@google.com> wrote:
> On Mon, Jan 16, 2012 at 10:36 AM, David Symonds <dsymonds@golang.org> wrote:
>> Wouldn't it be better to fix godoc instead?
>
> I cannot decide if godoc is broken.
A mail to golang-dev/golang-nuts would be a good way to start that discussion.
Sign in to reply to this message.
crawshaw1
On Mon, Jan 16, 2012 at 10:48 AM, David Symonds <dsymonds@golang.org> wrote: > On Mon, ...
13 years, 12 months ago (2012年01月15日 23:57:28 UTC) #5
On Mon, Jan 16, 2012 at 10:48 AM, David Symonds <dsymonds@golang.org> wrote:
> On Mon, Jan 16, 2012 at 10:43 AM, David Crawshaw <crawshaw@google.com> wrote:
>
>> On Mon, Jan 16, 2012 at 10:36 AM, David Symonds <dsymonds@golang.org> wrote:
>>> Wouldn't it be better to fix godoc instead?
>>
>> I cannot decide if godoc is broken.
>
> A mail to golang-dev/golang-nuts would be a good way to start that discussion.
Done. However, I think this CL stands on its own. There are other
examples of sets of similar public variables not having a doc line
each (e.g. net/http), and I think the godoc is much prettier if the
Model implementations are in one variable block.
Sign in to reply to this message.
dsymonds
On Mon, Jan 16, 2012 at 10:57 AM, David Crawshaw <crawshaw@google.com> wrote: > Done. However, ...
13 years, 12 months ago (2012年01月16日 00:09:00 UTC) #6
On Mon, Jan 16, 2012 at 10:57 AM, David Crawshaw <crawshaw@google.com> wrote:
> Done. However, I think this CL stands on its own. There are other
> examples of sets of similar public variables not having a doc line
> each (e.g. net/http), and I think the godoc is much prettier if the
> Model implementations are in one variable block.
The question is though, would you make the change in this CL if godoc
behaved differently? We shouldn't distort code just because a tool
mishandles it.
Dave.
Sign in to reply to this message.
adg
On 16 January 2012 11:09, David Symonds <dsymonds@golang.org> wrote: > On Mon, Jan 16, 2012 ...
13 years, 12 months ago (2012年01月16日 00:11:02 UTC) #7
On 16 January 2012 11:09, David Symonds <dsymonds@golang.org> wrote:
> On Mon, Jan 16, 2012 at 10:57 AM, David Crawshaw <crawshaw@google.com> wrote:
>
>> Done. However, I think this CL stands on its own. There are other
>> examples of sets of similar public variables not having a doc line
>> each (e.g. net/http), and I think the godoc is much prettier if the
>> Model implementations are in one variable block.
>
> The question is though, would you make the change in this CL if godoc
> behaved differently? We shouldn't distort code just because a tool
> mishandles it.
I agree with Crawshaw here. The code is definitely better now
regardless of godoc.
Andrew
Sign in to reply to this message.
dsymonds
On Mon, Jan 16, 2012 at 11:10 AM, Andrew Gerrand <adg@golang.org> wrote: > I agree ...
13 years, 12 months ago (2012年01月16日 00:12:42 UTC) #8
On Mon, Jan 16, 2012 at 11:10 AM, Andrew Gerrand <adg@golang.org> wrote:
> I agree with Crawshaw here. The code is definitely better now
> regardless of godoc.
I haven't looked closely at the code. If it looks better regardless of
godoc changes, that's fine with me.
Sign in to reply to this message.
nigeltao
http://codereview.appspot.com/5544073/diff/4003/src/pkg/image/color/color.go File src/pkg/image/color/color.go (right): http://codereview.appspot.com/5544073/diff/4003/src/pkg/image/color/color.go#newcode167 src/pkg/image/color/color.go:167: func modelRGBA(c Color) Color { s/modelRGBA/rgbaModel/ and similarly elsewhere.
13 years, 12 months ago (2012年01月16日 03:35:16 UTC) #9
http://codereview.appspot.com/5544073/diff/4003/src/pkg/image/color/color.go
File src/pkg/image/color/color.go (right):
http://codereview.appspot.com/5544073/diff/4003/src/pkg/image/color/color.go#...
src/pkg/image/color/color.go:167: func modelRGBA(c Color) Color {
s/modelRGBA/rgbaModel/ and similarly elsewhere.
Sign in to reply to this message.
crawshaw1
http://codereview.appspot.com/5544073/diff/4003/src/pkg/image/color/color.go File src/pkg/image/color/color.go (right): http://codereview.appspot.com/5544073/diff/4003/src/pkg/image/color/color.go#newcode167 src/pkg/image/color/color.go:167: func modelRGBA(c Color) Color { On 2012年01月16日 03:35:17, nigeltao ...
13 years, 12 months ago (2012年01月16日 04:34:45 UTC) #10
http://codereview.appspot.com/5544073/diff/4003/src/pkg/image/color/color.go
File src/pkg/image/color/color.go (right):
http://codereview.appspot.com/5544073/diff/4003/src/pkg/image/color/color.go#...
src/pkg/image/color/color.go:167: func modelRGBA(c Color) Color {
On 2012年01月16日 03:35:17, nigeltao wrote:
> s/modelRGBA/rgbaModel/ and similarly elsewhere.
Done. (I was avoiding rGBAModel the hard way.)
Sign in to reply to this message.
nigeltao
LGTM.
13 years, 12 months ago (2012年01月16日 05:00:27 UTC) #11
LGTM.
Sign in to reply to this message.
nigeltao
*** Submitted as http://code.google.com/p/go/source/detail?r=c2f40bab9029 *** image/color: simplify documentation R=nigeltao, dsymonds, adg CC=golang-dev http://codereview.appspot.com/5544073 Committer: Nigel ...
13 years, 12 months ago (2012年01月16日 05:02:51 UTC) #12
*** Submitted as http://code.google.com/p/go/source/detail?r=c2f40bab9029 ***
image/color: simplify documentation
R=nigeltao, dsymonds, adg
CC=golang-dev
http://codereview.appspot.com/5544073
Committer: Nigel Tao <nigeltao@golang.org>
Sign in to reply to this message.
|
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /