Merge lp:~dave-cheney/goose/100-fix-vet-errors into lp:goose

Proposed by Dave Cheney on 2014-05-16
Status: Merged
Approved by: Dave Cheney on 2014-05-16
Approved revision: 127
Merged at revision: 124
Proposed branch: lp:~dave-cheney/goose/100-fix-vet-errors
Merge into: lp:goose
Diff against target: 123 lines (+19/-25)
6 files modified
nova/json.go (+8/-9)
testservices/cmd/main.go (+1/-2)
testservices/identityservice/keypair.go (+4/-6)
testservices/identityservice/userpass.go (+4/-6)
testservices/novaservice/service_http.go (+1/-1)
tools/secgroup-delete-all/main_test.go (+1/-1)
To merge this branch: bzr merge lp:~dave-cheney/goose/100-fix-vet-errors
Reviewer Review Type Date Requested Status
Juju Engineering 2014-05-16 Pending
Review via email: mp+219781@code.launchpad.net

Commit message

goose: fix vet errors

* fix several printf errors
* address unreachable code warnings. Rewrote branches to avoid unreachable code while being compatible with Go 1.0

Description of the change

goose: fix vet errors

* fix several printf errors
* address unreachable code warnings. Rewrote branches to avoid unreachable code while being compatible with Go 1.0

https://codereview.appspot.com/96360045/

To post a comment you must log in.
Dave Cheney (dave-cheney) wrote :
Download full text (5.2 KiB)

Reviewers: mp+219781_code.launchpad.net,

Message:
Please take a look.

Description:
goose: fix vet errors

* fix several printf errors
* address unreachable code warnings. Rewrote branches to avoid
unreachable code while being compatible with Go 1.0

https://code.launchpad.net/~dave-cheney/goose/100-fix-vet-errors/+merge/219781

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/96360045/

Affected files (+21, -25 lines):
   A [revision details]
   M nova/json.go
   M testservices/cmd/main.go
   M testservices/identityservice/keypair.go
   M testservices/identityservice/userpass.go
   M testservices/novaservice/service_http.go
   M tools/secgroup-delete-all/main_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140513085629-73yp1ik7s2wh2u5o
+New revision: <email address hidden>

Index: nova/json.go
=== modified file 'nova/json.go'
--- nova/json.go 2013-06-26 02:55:42 +0000
+++ nova/json.go 2014-05-16 06:16:12 +0000
@@ -43,16 +43,15 @@
   if err := json.Unmarshal(b, &out); err != nil {
    return "", err
   }
- if val, ok := out[tag]; !ok {
+ val, ok := out[tag]
+ if !ok {
    return "", nil
- } else {
- if floatVal, ok := val.(float64); ok {
- return fmt.Sprint(int(floatVal)), nil
- } else {
- return fmt.Sprint(val), nil
- }
- }
- panic("unreachable")
+ }
+ floatVal, ok := val.(float64)
+ if ok {
+ return fmt.Sprint(int(floatVal)), nil
+ }
+ return fmt.Sprint(val), nil
  }

  // appendJSON marshals the given attribute value and appends it as an
encoded value to the given json data.

Index: testservices/cmd/main.go
=== modified file 'testservices/cmd/main.go'
--- testservices/cmd/main.go 2013-01-24 03:15:19 +0000
+++ testservices/cmd/main.go 2014-05-16 06:10:03 +0000
@@ -38,7 +38,6 @@
  var users userValues

  func init() {
-
   gnuflag.Var(&users, "user", "supply to add a user to the identity
provider. Can be supplied multiple times. Should be of the form
\"user:secret:token\".")
  }

@@ -59,7 +58,7 @@
   gnuflag.Parse(true)
   p, ok := providerMap[*provider]
   if !ok {
- log.Fatalf("No such provider: %s, pick one of: %v", provider,
providers())
+ log.Fatalf("No such provider: %s, pick one of: %v", *provider,
providers())
   }
   mux := http.NewServeMux()
   p.SetupHTTP(mux)

Index: testservices/identityservice/keypair.go
=== modified file 'testservices/identityservice/keypair.go'
--- testservices/identityservice/keypair.go 2013-05-21 01:07:40 +0000
+++ testservices/identityservice/keypair.go 2014-05-16 06:12:51 +0000
@@ -90,15 +90,13 @@
    u.ReturnFailure(w, http.StatusInternalServerError, err.Error())
    return
   }
- if content, err := json.Marshal(res); err != nil {
+ content, err := json.Marshal(res)
+ if err != nil {
    u.ReturnFailure(w, http.StatusInternalServerError, err.Error())
    return
- } else {
- w.WriteHeader(http.StatusOK)
- w.Write(content)
- return
   }
- panic("unreachable")
+ w.WriteHeader(http.StatusOK)
+ w.Write(content)
  }

  func (u *KeyPair) ...

Read more...

Andrew Wilkins (axwalk) wrote :

On 2014/05/16 06:17:54, dfc wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/96360045/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/json.go'
2--- nova/json.go 2013-06-26 02:55:42 +0000
3+++ nova/json.go 2014-05-16 06:17:31 +0000
4@@ -43,16 +43,15 @@
5 if err := json.Unmarshal(b, &out); err != nil {
6 return "", err
7 }
8- if val, ok := out[tag]; !ok {
9+ val, ok := out[tag]
10+ if !ok {
11 return "", nil
12- } else {
13- if floatVal, ok := val.(float64); ok {
14- return fmt.Sprint(int(floatVal)), nil
15- } else {
16- return fmt.Sprint(val), nil
17- }
18- }
19- panic("unreachable")
20+ }
21+ floatVal, ok := val.(float64)
22+ if ok {
23+ return fmt.Sprint(int(floatVal)), nil
24+ }
25+ return fmt.Sprint(val), nil
26 }
27
28 // appendJSON marshals the given attribute value and appends it as an encoded value to the given json data.
29
30=== modified file 'testservices/cmd/main.go'
31--- testservices/cmd/main.go 2013-01-24 03:15:19 +0000
32+++ testservices/cmd/main.go 2014-05-16 06:17:31 +0000
33@@ -38,7 +38,6 @@
34 var users userValues
35
36 func init() {
37-
38 gnuflag.Var(&users, "user", "supply to add a user to the identity provider. Can be supplied multiple times. Should be of the form \"user:secret:token\".")
39 }
40
41@@ -59,7 +58,7 @@
42 gnuflag.Parse(true)
43 p, ok := providerMap[*provider]
44 if !ok {
45- log.Fatalf("No such provider: %s, pick one of: %v", provider, providers())
46+ log.Fatalf("No such provider: %s, pick one of: %v", *provider, providers())
47 }
48 mux := http.NewServeMux()
49 p.SetupHTTP(mux)
50
51=== modified file 'testservices/identityservice/keypair.go'
52--- testservices/identityservice/keypair.go 2013-05-21 01:07:40 +0000
53+++ testservices/identityservice/keypair.go 2014-05-16 06:17:31 +0000
54@@ -90,15 +90,13 @@
55 u.ReturnFailure(w, http.StatusInternalServerError, err.Error())
56 return
57 }
58- if content, err := json.Marshal(res); err != nil {
59+ content, err := json.Marshal(res)
60+ if err != nil {
61 u.ReturnFailure(w, http.StatusInternalServerError, err.Error())
62 return
63- } else {
64- w.WriteHeader(http.StatusOK)
65- w.Write(content)
66- return
67 }
68- panic("unreachable")
69+ w.WriteHeader(http.StatusOK)
70+ w.Write(content)
71 }
72
73 func (u *KeyPair) generateAccessResponse(userInfo *UserInfo) (*AccessResponse, error) {
74
75=== modified file 'testservices/identityservice/userpass.go'
76--- testservices/identityservice/userpass.go 2013-06-24 04:26:54 +0000
77+++ testservices/identityservice/userpass.go 2014-05-16 06:17:31 +0000
78@@ -222,15 +222,13 @@
79 u.ReturnFailure(w, http.StatusInternalServerError, err.Error())
80 return
81 }
82- if content, err := json.Marshal(res); err != nil {
83+ content, err := json.Marshal(res)
84+ if err != nil {
85 u.ReturnFailure(w, http.StatusInternalServerError, err.Error())
86 return
87- } else {
88- w.WriteHeader(http.StatusOK)
89- w.Write(content)
90- return
91 }
92- panic("All paths should have already returned")
93+ w.WriteHeader(http.StatusOK)
94+ w.Write(content)
95 }
96
97 func (u *UserPass) generateAccessResponse(userInfo *UserInfo) (*AccessResponse, error) {
98
99=== modified file 'testservices/novaservice/service_http.go'
100--- testservices/novaservice/service_http.go 2014-05-02 01:12:04 +0000
101+++ testservices/novaservice/service_http.go 2014-05-16 06:17:31 +0000
102@@ -898,7 +898,7 @@
103 // TODO: Use a proper helper and sane error type
104 return &errorResponse{
105 http.StatusBadRequest,
106- fmt.Sprintf(`{"badRequest": {"message": "This rule already exists in group %d", "code": 400}}`, group.Id),
107+ fmt.Sprintf(`{"badRequest": {"message": "This rule already exists in group %s", "code": 400}}`, group.Id),
108 "application/json; charset=UTF-8",
109 "rule already exists",
110 nil,
111
112=== modified file 'tools/secgroup-delete-all/main_test.go'
113--- tools/secgroup-delete-all/main_test.go 2014-05-01 18:04:32 +0000
114+++ tools/secgroup-delete-all/main_test.go 2014-05-16 06:17:31 +0000
115@@ -75,7 +75,7 @@
116 func deleteGroupError(s hook.ServiceControl, args ...interface{}) error {
117 groupId := args[0].(string)
118 if groupId == doNotDelete.Id {
119- return fmt.Errorf("cannot delete group %d", groupId)
120+ return fmt.Errorf("cannot delete group %s", groupId)
121 }
122 return nil
123 }

Subscribers

People subscribed via source and target branches