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

Issue 8585043: charm: use functions for validation

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by niemeyer
Modified:
11 years ago
Reviewers:
Danilo, dimitern, mp+157924
Visibility:
Public.

Description

charm: use functions for validation https://code.launchpad.net/~niemeyer/juju-core/func-validators/+merge/157924 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 4

Patch Set 2 : charm: use functions for validation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+66 lines, -55 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M charm/url.go View 4 chunks +21 lines, -11 lines 0 comments Download
M charm/url_test.go View 1 2 chunks +42 lines, -43 lines 0 comments Download
M cmd/juju/bootstrap.go View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4
niemeyer
Please take a look.
11 years ago (2013-04-09 17:27:12 UTC) #1
dimitern
I like it! LGTM
11 years ago (2013-04-09 17:43:24 UTC) #2
Danilo
LGTM with a tiny suggestion on variable naming. https://codereview.appspot.com/8585043/diff/1/charm/url_test.go File charm/url_test.go (left): https://codereview.appspot.com/8585043/diff/1/charm/url_test.go#oldcode121 charm/url_test.go:121: var ...
11 years ago (2013-04-09 18:16:37 UTC) #3
niemeyer
11 years ago (2013-04-09 18:28:48 UTC) #4
*** Submitted:

charm: use functions for validation

R=dimitern, danilo.segan
CC=
https://codereview.appspot.com/8585043

https://codereview.appspot.com/8585043/diff/1/charm/url_test.go
File charm/url_test.go (left):

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.
Sign in to reply to this message.

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