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

Issue 7634045: code review 7634045: database/sql: add DB.SetMaxIdleConns (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by bradfitz
Modified:
10 years, 11 months ago
Reviewers:
snaury
CC:
golang-dev, r
Visibility:
Public.

Description

database/sql: add DB.SetMaxIdleConns Update issue 4805

Patch Set 1 #

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

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

Total comments: 3

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

Total comments: 4

Patch Set 5 : diff -r 7a70129d0a6e https://go.googlecode.com/hg/ #

Patch Set 6 : diff -r 172ed4b6cbc9 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+70 lines, -6 lines) Patch
M src/pkg/database/sql/sql.go View 1 2 3 4 3 chunks +41 lines, -6 lines 0 comments Download
M src/pkg/database/sql/sql_test.go View 1 2 3 1 chunk +29 lines, -0 lines 0 comments Download

Messages

Total messages: 15
bradfitz
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
11 years ago (2013-03-18 19:50:02 UTC) #1
r
https://codereview.appspot.com/7634045/diff/4001/src/pkg/database/sql/sql.go File src/pkg/database/sql/sql.go (right): https://codereview.appspot.com/7634045/diff/4001/src/pkg/database/sql/sql.go#newcode183 src/pkg/database/sql/sql.go:183: // session state and observing that state is required, ...
11 years ago (2013-03-18 20:19:51 UTC) #2
bradfitz
On Mon, Mar 18, 2013 at 1:19 PM, <r@golang.org> wrote: > > https://codereview.appspot.**com/7634045/diff/4001/src/pkg/** > database/sql/sql.go<https://codereview.appspot.com/7634045/diff/4001/src/pkg/database/sql/sql.go> ...
11 years ago (2013-03-18 20:39:42 UTC) #3
r
https://codereview.appspot.com/7634045/diff/4001/src/pkg/database/sql/sql.go File src/pkg/database/sql/sql.go (right): https://codereview.appspot.com/7634045/diff/4001/src/pkg/database/sql/sql.go#newcode183 src/pkg/database/sql/sql.go:183: // session state and observing that state is required, ...
11 years ago (2013-03-18 21:03:02 UTC) #4
r
On Mon, Mar 18, 2013 at 1:39 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > Yes. The ...
11 years ago (2013-03-18 21:03:21 UTC) #5
bradfitz
Hello golang-dev@googlegroups.com, r@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
11 years ago (2013-03-18 21:47:16 UTC) #6
bradfitz
PTAL Just the setter method here. Removed the unrelated English changes. On Mon, Mar 18, ...
11 years ago (2013-03-18 21:48:10 UTC) #7
r
https://codereview.appspot.com/7634045/diff/13001/src/pkg/database/sql/sql.go File src/pkg/database/sql/sql.go (right): https://codereview.appspot.com/7634045/diff/13001/src/pkg/database/sql/sql.go#newcode351 src/pkg/database/sql/sql.go:351: // SetMaxIdleConns controls the size of the idle connection ...
11 years ago (2013-03-18 21:53:45 UTC) #8
bradfitz
https://codereview.appspot.com/7634045/diff/13001/src/pkg/database/sql/sql.go File src/pkg/database/sql/sql.go (right): https://codereview.appspot.com/7634045/diff/13001/src/pkg/database/sql/sql.go#newcode351 src/pkg/database/sql/sql.go:351: // SetMaxIdleConns controls the size of the idle connection ...
11 years ago (2013-03-18 22:12:02 UTC) #9
bradfitz
Hello golang-dev@googlegroups.com, r@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
11 years ago (2013-03-18 22:15:03 UTC) #10
bradfitz
PTAL. Now with more explicit comments and some of your updated wording. On Mon, Mar ...
11 years ago (2013-03-18 22:15:57 UTC) #11
r
LGTM
11 years ago (2013-03-18 22:22:00 UTC) #12
bradfitz
*** Submitted as https://code.google.com/p/go/source/detail?r=1130cc72c645 *** database/sql: add DB.SetMaxIdleConns Update issue 4805 R=golang-dev, r CC=golang-dev https://codereview.appspot.com/7634045
11 years ago (2013-03-18 22:33:06 UTC) #13
snaury
On 2013/03/18 22:33:06, bradfitz wrote: > *** Submitted as https://code.google.com/p/go/source/detail?r=1130cc72c645 *** > > database/sql: add ...
10 years, 11 months ago (2013-04-20 11:39:55 UTC) #14
snaury
10 years, 11 months ago (2013-04-20 11:42:16 UTC) #15
Message was sent while issue was closed.
On 2013/04/20 11:39:55, snaury wrote:
> Please see issue 4805 (https://code.google.com/p/go/issues/detail?id=4805),
this
> CL breaks prepared statements.

Oops, sorry for the noise, it doesn't. The 2 connections default was before as
well. %)
Sign in to reply to this message.

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