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

Issue 5695073: Adoption of gocheck changes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 2 months ago by TheMue
Modified:
12 years, 1 month ago
Reviewers:
mp+94750, niemeyer, fwereade, rog
Visibility:
Public.

Description

After the API of gocheck has changed the tests must adopt these changes. This proposal contains the changes for the state package. https://code.launchpad.net/~themue/juju/go-state-gocheck-change/+merge/94750 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 1

Patch Set 2 : Adoption of gocheck changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+74 lines, -74 lines) Patch
M state/internal_test.go View 1 19 chunks +68 lines, -68 lines 0 comments Download
M state/state_test.go View 1 4 chunks +6 lines, -6 lines 0 comments Download

Messages

Total messages: 6
TheMue
Please take a look.
12 years, 2 months ago (2012-02-27 11:34:14 UTC) #1
fwereade
I'm pretty sure you're missing at least one Assert(len(foo), Equals, bar). (should ideally use HasLen)
12 years, 2 months ago (2012-02-27 11:36:35 UTC) #2
rog
LGTM https://codereview.appspot.com/5695073/diff/1/state/internal_test.go File state/internal_test.go (right): https://codereview.appspot.com/5695073/diff/1/state/internal_test.go#newcode691 state/internal_test.go:691: ItemChange{ItemModified, "foo", "bar", "different"}, i know this isn't ...
12 years, 2 months ago (2012-02-27 11:40:49 UTC) #3
TheMue
Please take a look.
12 years, 2 months ago (2012-02-27 11:59:41 UTC) #4
fwereade
Thanks, LGTM.
12 years, 2 months ago (2012-02-27 12:01:26 UTC) #5
niemeyer
12 years, 1 month ago (2012-03-09 11:06:15 UTC) #6
LGTM
Sign in to reply to this message.

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