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

Issue 6637049: jujuc: extract HookContext

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 7 months ago by fwereade
Modified:
11 years, 7 months ago
Reviewers:
mp+128641
Visibility:
Public.

Description

jujuc: extract HookContext Not clean yet, but big enough already; followups will (1) remove the HookContext dependency in the jujuc tests and (2) fix up HookContext so that it doesn't have messy overlapping public fields. https://code.launchpad.net/~fwereade/juju-core/jujuc-extract-hook-context/+merge/128641 Requires: https://code.launchpad.net/~fwereade/juju-core/jujuc-use-context-interface/+merge/128525 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 1

Patch Set 2 : jujuc: extract HookContext #

Unified diffs Side-by-side diffs Delta from patch set Stats (+285 lines, -209 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M worker/uniter/context.go View 1 14 chunks +21 lines, -102 lines 0 comments Download
M worker/uniter/context_test.go View 1 12 chunks +76 lines, -51 lines 0 comments Download
M worker/uniter/jujuc/config-get_test.go View 5 chunks +5 lines, -4 lines 0 comments Download
M worker/uniter/jujuc/context.go View 1 2 chunks +51 lines, -0 lines 0 comments Download
M worker/uniter/jujuc/juju-log_test.go View 3 chunks +8 lines, -7 lines 0 comments Download
M worker/uniter/jujuc/ports_test.go View 5 chunks +5 lines, -4 lines 0 comments Download
M worker/uniter/jujuc/relation-get_test.go View 1 5 chunks +5 lines, -4 lines 0 comments Download
M worker/uniter/jujuc/relation-ids_test.go View 6 chunks +8 lines, -7 lines 0 comments Download
M worker/uniter/jujuc/relation-list_test.go View 4 chunks +5 lines, -4 lines 0 comments Download
M worker/uniter/jujuc/relation-set_test.go View 4 chunks +4 lines, -4 lines 0 comments Download
M worker/uniter/jujuc/server.go View 1 chunk +33 lines, -0 lines 0 comments Download
M worker/uniter/jujuc/server_test.go View 1 chunk +38 lines, -0 lines 0 comments Download
M worker/uniter/jujuc/unit-get_test.go View 6 chunks +6 lines, -5 lines 0 comments Download
M worker/uniter/jujuc/util_test.go View 5 chunks +11 lines, -9 lines 0 comments Download
M worker/uniter/relationer.go View 3 chunks +5 lines, -6 lines 0 comments Download
M worker/uniter/uniter.go View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 3
fwereade
Please take a look.
11 years, 7 months ago (2012-10-09 07:47:45 UTC) #1
niemeyer
LGTM. Nicely straightforward. https://codereview.appspot.com/6637049/diff/1/worker/uniter/jujuc/context.go File worker/uniter/jujuc/context.go (right): https://codereview.appspot.com/6637049/diff/1/worker/uniter/jujuc/context.go#newcode104 worker/uniter/jujuc/context.go:104: r := ctx.HookRelation() Note Has+Get on ...
11 years, 7 months ago (2012-10-09 19:32:40 UTC) #2
fwereade
11 years, 7 months ago (2012-10-09 21:14:36 UTC) #3
*** Submitted:

jujuc: extract HookContext

Not clean yet, but big enough already; followups will (1) remove the
HookContext dependency in the jujuc tests and (2) fix up HookContext
so that it doesn't have messy overlapping public fields.

R=niemeyer
CC=
https://codereview.appspot.com/6637049
Sign in to reply to this message.

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