juju-core

Merge lp://staging/~niemeyer/juju-core/func-validators into lp://staging/~juju/juju-core/trunk

Proposed by Gustavo Niemeyer on 2013-04-09

Status:	Merged
Merged at revision:	1133
Proposed branch:	lp://staging/~niemeyer/juju-core/func-validators
Merge into:	lp://staging/~juju/juju-core/trunk
Diff against target:	184 lines (+62/-53) 3 files modified charm/url.go (+21/-11) charm/url_test.go (+40/-41) cmd/juju/bootstrap.go (+1/-1)
To merge this branch:	bzr merge lp://staging/~niemeyer/juju-core/func-validators
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju Engineering		2013-04-09	Pending
Review via email: mp+157924@code.staging.launchpad.net

charm: use functions for validation

Revision history for this message

Gustavo Niemeyer (niemeyer) wrote on 2013-04-09:

Reviewers: mp+157924_code.launchpad.net,

Message:
Please take a look.

Description:
charm: use functions for validation

(do not edit description out of merge proposal)

Affected files:
   A [revision details]
   M charm/url.go
   M charm/url_test.go
   M cmd/juju/bootstrap.go

Revision history for this message

Dimiter Naydenov (dimitern) wrote on 2013-04-09:

I like it! LGTM

Revision history for this message

Данило Шеган (danilo) wrote on 2013-04-09:

LGTM with a tiny suggestion on variable naming.

https://codereview.appspot.com/8585043/diff/1/charm/url_test.go#oldcode121
charm/url_test.go:121: var validRegexpTests = []struct {
It's probably worthwhile to rename this to validCheckerTests as well
(unless I am missing something).

Revision history for this message

Gustavo Niemeyer (niemeyer) wrote on 2013-04-09:

*** Submitted:

charm: use functions for validation

https://codereview.appspot.com/8585043/diff/1/charm/url_test.go#oldcode121
charm/url_test.go:121: var validRegexpTests = []struct {
On 2013/04/09 18:16:37, danilo.segan wrote:
> It's probably worthwhile to rename this to validCheckerTests as well
(unless I
> am missing something).

Indeed, thanks.

https://codereview.appspot.com/8585043/diff/1/charm/url_test.go#oldcode166
charm/url_test.go:166: for i, t := range validRegexpTests {
On 2013/04/09 18:16:37, danilo.segan wrote:
> See above.

Done.