Issue 6298081: state: Moved function AssignUnit() to method of State.

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 6298081: state: Moved function AssignUnit() to method of State. (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
11 years, 11 months ago by TheMue

Modified:
11 years, 11 months ago

Reviewers:
mp+110302

Visibility:
Public.

Description

state: Moved function AssignUnit() to method of State. Detected that AssignUnit() is pure function getting a state as first argument doesn't follow the style of the state package. Here we have methods for adding, retrieving or removing entities as methods of state. This also matches for AssignUnit(), so after a short discussion moved it to be a method of State. https://code.launchpad.net/~themue/juju-core/go-state-move-assign-unit/+merge/110302 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : state: Moved function AssignUnit() to method of State. #

Created: 11 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+36 lines, -34 lines)			Patch
A	[revision details]	View	1	1 chunk	+2 lines, -0 lines	0 comments	Download
M	state/state.go	View		1 chunk	+28 lines, -0 lines	0 comments	Download
M	state/state_test.go	View	1	4 chunks	+5 lines, -5 lines	0 comments	Download
M	state/unit.go	View	1	1 chunk	+0 lines, -28 lines	0 comments	Download
M	state/watcher.go	View	1	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

rog

On 2012/06/14 11:33:18, TheMue wrote: > Please take a look. LGTM

11 years, 11 months ago (2012-06-14 11:34:45 UTC) #2

niemeyer

Well reviewed already. Please feel free to move ahead and merge it, Frank.

11 years, 11 months ago (2012-06-15 01:18:36 UTC) #4

TheMue

11 years, 11 months ago (2012-06-15 07:34:29 UTC) #5

*** Submitted:

state: Moved function AssignUnit() to method of State.

Detected that AssignUnit() is pure function getting a state
as first argument doesn't follow the style of the state package.
Here we have methods for adding, retrieving or removing entities
as methods of state. This also matches for AssignUnit(), so after
a short discussion moved it to be a method of State.

R=rog, fwereade, niemeyer
CC=
https://codereview.appspot.com/6298081

Expand All Messages | Collapse All Messages