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

Issue 7460047: Add Resolved to the API

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by benji
Modified:
11 years ago
Reviewers:
dimitern, mp+151555, rog
Visibility:
Public.

Description

Add Resolved to the API https://code.launchpad.net/~benji/juju-core/1130173/+merge/151555 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 22

Patch Set 2 : Add Resolved to the API #

Patch Set 3 : Add Resolved to the API #

Unified diffs Side-by-side diffs Delta from patch set Stats (+689 lines, -6 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/resolved.go View 1 2 chunks +6 lines, -4 lines 0 comments Download
M state/api/apiclient.go View 1 1 chunk +10 lines, -0 lines 0 comments Download
A state/api/apiserver.go.THIS View 1 1 chunk +532 lines, -0 lines 0 comments Download
M state/api/params/params.go View 1 1 chunk +13 lines, -0 lines 0 comments Download
M state/apiserver/api_test.go View 1 3 chunks +41 lines, -0 lines 0 comments Download
M state/apiserver/apiserver.go View 1 1 chunk +5 lines, -0 lines 0 comments Download
M state/machine_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
A state/statecmd/resolved.go View 1 1 chunk +34 lines, -0 lines 0 comments Download
A state/statecmd/resolved_test.go View 1 2 1 chunk +44 lines, -0 lines 0 comments Download
M state/unit_test.go View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 8
benji
Please take a look.
11 years ago (2013-03-04 16:51:09 UTC) #1
dimitern
Overall LGTM, just some trivials. https://codereview.appspot.com/7460047/diff/1/state/api/api_test.go File state/api/api_test.go (right): https://codereview.appspot.com/7460047/diff/1/state/api/api_test.go#newcode269 state/api/api_test.go:269: if err != nil ...
11 years ago (2013-03-05 22:45:27 UTC) #2
benji
Thanks for the review. I have integrated all your review comments. I had one or ...
11 years ago (2013-03-07 14:33:21 UTC) #3
dimitern
Thank you for the changes! I answered your questions below. https://codereview.appspot.com/7460047/diff/1/state/api/api_test.go File state/api/api_test.go (right): https://codereview.appspot.com/7460047/diff/1/state/api/api_test.go#newcode269 ...
11 years ago (2013-03-07 15:33:18 UTC) #4
fwereade
Code looks fine, but please fix the tests. https://codereview.appspot.com/7460047/diff/1/state/statecmd/resolved_test.go File state/statecmd/resolved_test.go (right): https://codereview.appspot.com/7460047/diff/1/state/statecmd/resolved_test.go#newcode18 state/statecmd/resolved_test.go:18: repo ...
11 years ago (2013-03-09 14:42:02 UTC) #5
rog
LGTM with the below ErrCode fix and other people's comments addressed. https://codereview.appspot.com/7460047/diff/1/state/api/api_test.go File state/api/api_test.go (right): ...
11 years ago (2013-03-09 18:01:08 UTC) #6
benji
I got all of the review comments integrated except for one non-critical one (but I ...
11 years ago (2013-03-12 17:37:24 UTC) #7
benji
11 years ago (2013-03-12 18:45:59 UTC) #8
*** Submitted:

Add Resolved to the API

R=dimitern, fwereade, rog
CC=
https://codereview.appspot.com/7460047
Sign in to reply to this message.

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