Code review comment for lp:~dave-cheney/goose/100-fix-vet-errors

Revision history for this message
Dave Cheney (dave-cheney) wrote :

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) generateAccessResponse(userInfo *UserInfo)
(*AccessResponse, error) {

Index: testservices/identityservice/userpass.go
=== modified file 'testservices/identityservice/userpass.go'
--- testservices/identityservice/userpass.go 2013-06-24 04:26:54 +0000
+++ testservices/identityservice/userpass.go 2014-05-16 06:12:51 +0000
@@ -222,15 +222,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("All paths should have already returned")
+ w.WriteHeader(http.StatusOK)
+ w.Write(content)
  }

  func (u *UserPass) generateAccessResponse(userInfo *UserInfo)
(*AccessResponse, error) {

Index: testservices/novaservice/service_http.go
=== modified file 'testservices/novaservice/service_http.go'
--- testservices/novaservice/service_http.go 2014-05-02 01:12:04 +0000
+++ testservices/novaservice/service_http.go 2014-05-16 06:14:14 +0000
@@ -898,7 +898,7 @@
      // TODO: Use a proper helper and sane error type
      return &errorResponse{
       http.StatusBadRequest,
- fmt.Sprintf(`{"badRequest": {"message": "This rule already exists in
group %d", "code": 400}}`, group.Id),
+ fmt.Sprintf(`{"badRequest": {"message": "This rule already exists in
group %s", "code": 400}}`, group.Id),
       "application/json; charset=UTF-8",
       "rule already exists",
       nil,

Index: tools/secgroup-delete-all/main_test.go
=== modified file 'tools/secgroup-delete-all/main_test.go'
--- tools/secgroup-delete-all/main_test.go 2014-05-01 18:04:32 +0000
+++ tools/secgroup-delete-all/main_test.go 2014-05-16 06:14:14 +0000
@@ -75,7 +75,7 @@
  func deleteGroupError(s hook.ServiceControl, args ...interface{}) error {
   groupId := args[0].(string)
   if groupId == doNotDelete.Id {
- return fmt.Errorf("cannot delete group %d", groupId)
+ return fmt.Errorf("cannot delete group %s", groupId)
   }
   return nil
  }

« Back to merge proposal