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
(51)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 44680043: code review 44680043: sync: explain Pool's intentions (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years ago by r
Modified:
12 years ago
Reviewers:
rsc
CC:
golang-codereviews, gobot, rsc
Visibility:
Public.
sync: explain Pool's intentions Expand the type's doc comment to make its purpose clear and discourage misuse.

Patch Set 1 #

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

Total comments: 1

Patch Set 3 : diff -r fdfc321546e7 https://code.google.com/p/go/ #

Patch Set 4 : diff -r fdfc321546e7 https://code.google.com/p/go/ #

Total comments: 2

Patch Set 5 : diff -r 3ac4abe0c760 https://code.google.com/p/go/ #

Created: 12 years ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -1 line) Patch
M src/pkg/sync/pool.go View 1 2 3 4 1 chunk +11 lines, -1 line 0 comments Download
Total messages: 6
|
r
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
12 years ago (2013年12月20日 18:02:05 UTC) #1
Hello golang-codereviews@googlegroups.com,
I'd like you to review this change to
https://code.google.com/p/go/ 
Sign in to reply to this message.
gobot
R=rsc@golang.org (assigned by r@golang.org)
12 years ago (2013年12月20日 18:06:05 UTC) #2
R=rsc@golang.org (assigned by r@golang.org)
Sign in to reply to this message.
rsc
https://codereview.appspot.com/44680043/diff/20001/src/pkg/sync/pool.go File src/pkg/sync/pool.go (right): https://codereview.appspot.com/44680043/diff/20001/src/pkg/sync/pool.go#newcode17 src/pkg/sync/pool.go:17: // Pool is not intended for general-purpose free lists. ...
12 years ago (2013年12月20日 18:13:46 UTC) #3
https://codereview.appspot.com/44680043/diff/20001/src/pkg/sync/pool.go
File src/pkg/sync/pool.go (right):
https://codereview.appspot.com/44680043/diff/20001/src/pkg/sync/pool.go#newco...
src/pkg/sync/pool.go:17: // Pool is not intended for general-purpose free lists.
Instead, its
i think i'd lead with what it's for instead of what it's not for.
some other minor tweaks (good/not appropriate -> appropriate/not appropriate; to
be shared -> shared)
// Pool's intended use is for free lists maintained in global variables,
typically accessed
// by multiple goroutines simultaneously. Using a Pool instead of, say, a list
allows the
// runtime to reclaim entries from the free list when it makes sense to do so.
// An appropriate use of sync.Pool is to create a pool of temporary buffers
shared between
// independent clients of a global resource. On the other hand, if a free list
is maintained
// as part of an object used only by a single client and freed when the client
completes,
// implementing that free list as a Pool is not appropriate.
Sign in to reply to this message.
r
Hello golang-codereviews@googlegroups.com, gobot@golang.org, rsc@golang.org (cc: golang-codereviews@googlegroups.com), Please take another look.
12 years ago (2013年12月20日 18:33:35 UTC) #4
Sign in to reply to this message.
rsc
LGTM https://codereview.appspot.com/44680043/diff/60001/src/pkg/sync/pool.go File src/pkg/sync/pool.go (right): https://codereview.appspot.com/44680043/diff/60001/src/pkg/sync/pool.go#newcode20 src/pkg/sync/pool.go:20: // entries from the pool when it makes ...
12 years ago (2013年12月20日 19:09:51 UTC) #5
LGTM
https://codereview.appspot.com/44680043/diff/60001/src/pkg/sync/pool.go
File src/pkg/sync/pool.go (right):
https://codereview.appspot.com/44680043/diff/60001/src/pkg/sync/pool.go#newco...
src/pkg/sync/pool.go:20: // entries from the pool when it makes sense to do so. 
An
spacespace -> space
https://codereview.appspot.com/44680043/diff/60001/src/pkg/sync/pool.go#newco...
src/pkg/sync/pool.go:27: // This is an experimental package and might not be
released.
s/package/type/
Sign in to reply to this message.
r
*** Submitted as https://code.google.com/p/go/source/detail?r=d9681a334cc4 *** sync: explain Pool's intentions Expand the type's doc comment to ...
12 years ago (2013年12月20日 19:15:53 UTC) #6
*** Submitted as https://code.google.com/p/go/source/detail?r=d9681a334cc4 ***
sync: explain Pool's intentions
Expand the type's doc comment to make its purpose clear
and discourage misuse.
R=golang-codereviews, gobot, rsc
CC=golang-codereviews
https://codereview.appspot.com/44680043 
Sign in to reply to this message.
|
Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b

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