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

Issue 6099051: Go port for local provider networking

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 11 months ago by andrewsmedina
Modified:
11 years, 10 months ago
Reviewers:
rog, niemeyer, fwereade, mp+103333
Visibility:
Public.

Description

Go port for local provider networking https://code.launchpad.net/~andrewsmedina/juju/go-local-network/+merge/103333 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 41

Patch Set 2 : Go port for local provider networking #

Total comments: 7

Patch Set 3 : Go port for local provider networking #

Total comments: 8

Patch Set 4 : Go port for local provider networking #

Total comments: 5

Patch Set 5 : Go port for local provider networking #

Unified diffs Side-by-side diffs Delta from patch set Stats (+294 lines, -0 lines) Patch
A [revision details] View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
A environs/local/network.go View 1 2 3 4 1 chunk +148 lines, -0 lines 0 comments Download
A environs/local/network_test.go View 1 2 3 4 1 chunk +144 lines, -0 lines 0 comments Download

Messages

Total messages: 14
andrewsmedina
Please take a look.
11 years, 11 months ago (2012-04-24 17:51:08 UTC) #1
rog
Looks plausible in general. These comments are based on a very shallow reading of the ...
11 years, 11 months ago (2012-04-26 14:50:43 UTC) #2
andrewsmedina
https://codereview.appspot.com/6099051/diff/1/environs/local/network.go File environs/local/network.go (right): https://codereview.appspot.com/6099051/diff/1/environs/local/network.go#newcode13 environs/local/network.go:13: type network struct { I'm using the loadAttributes method ...
11 years, 11 months ago (2012-04-27 01:18:03 UTC) #3
rog
some replies and a few more remarks. thanks for doing this BTW! https://codereview.appspot.com/6099051/diff/1/environs/local/network.go File environs/local/network.go ...
11 years, 11 months ago (2012-04-27 10:46:10 UTC) #4
andrewsmedina
Please take a look.
11 years, 10 months ago (2012-05-20 23:14:20 UTC) #5
niemeyer
A few minors, and I think this may go in. https://codereview.appspot.com/6099051/diff/7001/environs/local/network.go File environs/local/network.go (right): https://codereview.appspot.com/6099051/diff/7001/environs/local/network.go#newcode32 ...
11 years, 10 months ago (2012-05-22 00:17:45 UTC) #6
andrewsmedina
Please take a look.
11 years, 10 months ago (2012-05-22 04:53:39 UTC) #7
rog
very nearly there. thanks for making all of those changes! https://codereview.appspot.com/6099051/diff/1/environs/local/network.go File environs/local/network.go (right): https://codereview.appspot.com/6099051/diff/1/environs/local/network.go#newcode124 ...
11 years, 10 months ago (2012-05-22 10:41:16 UTC) #8
fwereade
LGTM with a little more testing and commenting as below https://codereview.appspot.com/6099051/diff/12001/environs/local/network.go File environs/local/network.go (right): https://codereview.appspot.com/6099051/diff/12001/environs/local/network.go#newcode12 ...
11 years, 10 months ago (2012-05-23 10:20:05 UTC) #9
andrewsmedina
Please take a look. https://codereview.appspot.com/6099051/diff/12001/environs/local/network_test.go File environs/local/network_test.go (right): https://codereview.appspot.com/6099051/diff/12001/environs/local/network_test.go#newcode1 environs/local/network_test.go:1: package local I also got ...
11 years, 10 months ago (2012-05-24 03:21:10 UTC) #10
rog
LGTM https://codereview.appspot.com/6099051/diff/15001/environs/local/network.go File environs/local/network.go (right): https://codereview.appspot.com/6099051/diff/15001/environs/local/network.go#newcode92 environs/local/network.go:92: // If network name does not exist, it ...
11 years, 10 months ago (2012-05-24 14:07:04 UTC) #11
niemeyer
Looks very nice. A few trivial and LGTM. https://codereview.appspot.com/6099051/diff/15001/environs/local/network.go File environs/local/network.go (right): https://codereview.appspot.com/6099051/diff/15001/environs/local/network.go#newcode12 environs/local/network.go:12: // ...
11 years, 10 months ago (2012-05-24 16:31:35 UTC) #12
niemeyer
Moving it back onto WIP to wait for these last details.
11 years, 10 months ago (2012-05-24 21:51:31 UTC) #13
andrewsmedina
11 years, 10 months ago (2012-05-24 23:07:35 UTC) #14
Please take a look.
Sign in to reply to this message.

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