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

Issue 6877054: First part of the nova service double. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 4 months ago by dimitern
Modified:
8 years, 4 months ago
Reviewers:
jameinel, mp+138115, wallyworld, rog
Visibility:
Public.

Description

First part of the nova service double. As discussed, I'm starting to propose bits of the nova service double in chunks, as I go. The direct API and its tests are ready. Added a comprehensive comment about RuleInfo struct, to describe better the types of security group rules. https://code.launchpad.net/~dimitern/goose/nova-testing-service/+merge/138115 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 87

Patch Set 2 : First part of the nova service double. #

Total comments: 45

Patch Set 3 : First part of the nova service double. #

Total comments: 2

Patch Set 4 : First part of the nova service double. #

Patch Set 5 : First part of the nova service double. #

Patch Set 6 : First part of the nova service double. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1618 lines, -14 lines) Patch
A [revision details] View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M nova/nova.go View 1 2 3 4 4 chunks +55 lines, -11 lines 0 comments Download
M nova/nova_test.go View 1 2 3 4 2 chunks +4 lines, -3 lines 0 comments Download
A testservices/novaservice/novaservice.go View 1 2 3 4 1 chunk +115 lines, -0 lines 0 comments Download
A testservices/novaservice/service.go View 1 2 3 4 1 chunk +501 lines, -0 lines 0 comments Download
A testservices/novaservice/service_http.go View 1 chunk +11 lines, -0 lines 0 comments Download
A testservices/novaservice/service_http_test.go View 1 1 chunk +33 lines, -0 lines 0 comments Download
A testservices/novaservice/service_test.go View 1 2 3 4 1 chunk +887 lines, -0 lines 0 comments Download
A testservices/novaservice/setup_test.go View 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 13
dimitern
Please take a look.
8 years, 4 months ago (2012-12-05 12:02:17 UTC) #1
wallyworld
This is coming along nicely. I'd like to see what the answers to the various ...
8 years, 4 months ago (2012-12-06 00:26:04 UTC) #2
jameinel
Overall LGTM. Some small bits to cleanup, but a lot of that could be done ...
8 years, 4 months ago (2012-12-06 06:07:10 UTC) #3
rog
this is looking nice. lots of comments, but not much substantial. https://codereview.appspot.com/6877054/diff/1/nova/nova.go File nova/nova.go (right): ...
8 years, 4 months ago (2012-12-06 17:32:40 UTC) #4
dimitern
Please take a look. https://codereview.appspot.com/6877054/diff/1/nova/nova.go File nova/nova.go (right): https://codereview.appspot.com/6877054/diff/1/nova/nova.go#newcode252 nova/nova.go:252: // Every rule is essentially ...
8 years, 4 months ago (2012-12-07 00:23:33 UTC) #5
wallyworld
LGTM with suggested fixes and rogpeppe's issues addressed https://codereview.appspot.com/6877054/diff/9001/testservices/novaservice/service.go File testservices/novaservice/service.go (right): https://codereview.appspot.com/6877054/diff/9001/testservices/novaservice/service.go#newcode88 testservices/novaservice/service.go:88: flavors ...
8 years, 4 months ago (2012-12-07 11:32:33 UTC) #6
rog
getting there! https://codereview.appspot.com/6877054/diff/1/testservices/novaservice/service.go File testservices/novaservice/service.go (right): https://codereview.appspot.com/6877054/diff/1/testservices/novaservice/service.go#newcode42 testservices/novaservice/service.go:42: func (n *Nova) AddFlavor(flavor Flavor) error { ...
8 years, 4 months ago (2012-12-07 12:15:34 UTC) #7
dfc
Getting close. Please consider what happens if the service double is accessed concurrently, at the ...
8 years, 4 months ago (2012-12-10 03:33:39 UTC) #8
dimitern
Please take a look. https://codereview.appspot.com/6877054/diff/9001/nova/nova.go File nova/nova.go (right): https://codereview.appspot.com/6877054/diff/9001/nova/nova.go#newcode63 nova/nova.go:63: Swap *string // Can be ...
8 years, 4 months ago (2012-12-10 12:46:09 UTC) #9
rog
LGTM with a couple of remarks below. https://codereview.appspot.com/6877054/diff/9001/nova/nova.go File nova/nova.go (right): https://codereview.appspot.com/6877054/diff/9001/nova/nova.go#newcode63 nova/nova.go:63: Swap *string ...
8 years, 4 months ago (2012-12-10 13:02:51 UTC) #10
dimitern
Please take a look. https://codereview.appspot.com/6877054/diff/9001/nova/nova.go File nova/nova.go (right): https://codereview.appspot.com/6877054/diff/9001/nova/nova.go#newcode63 nova/nova.go:63: Swap *string // Can be ...
8 years, 4 months ago (2012-12-10 14:05:44 UTC) #11
dimitern
Please take a look.
8 years, 4 months ago (2012-12-10 14:17:37 UTC) #12
dimitern
8 years, 4 months ago (2012-12-10 14:19:04 UTC) #13
*** Submitted:

First part of the nova service double.

As discussed, I'm starting to propose bits of the nova service double in chunks,
as I go.
The direct API and its tests are ready. Added a comprehensive comment about
RuleInfo struct,
to describe better the types of security group rules.

R=wallyworld, jameinel, rog, dfc
CC=
https://codereview.appspot.com/6877054
Sign in to reply to this message.

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