Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check errors #431

Merged
merged 17 commits into from
Dec 13, 2016
Merged

Check errors #431

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8285212
Added rakefile (original in github.com/Azure/autorest)
mcardosos Oct 11, 2016
6eb4fe0
Rakefile translated to golang
mcardosos Oct 18, 2016
00a0145
Updated travis file
mcardosos Oct 19, 2016
40fb143
Removed rakefile
mcardosos Oct 19, 2016
7478058
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-go
mcardosos Oct 20, 2016
e01c89c
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-go
mcardosos Oct 28, 2016
f177877
Included errcheck
mcardosos Oct 29, 2016
a7de7ad
Fix metadata
mcardosos Oct 31, 2016
1c94249
Removed panic
mcardosos Nov 1, 2016
9ca7400
Ignoring resp.body.Close() errors
mcardosos Dec 8, 2016
9048767
Discarding defer errors
mcardosos Dec 9, 2016
ef112da
Merge from master
mcardosos Dec 9, 2016
bf23f32
Fixed merged stuff
mcardosos Dec 9, 2016
ef160cf
Fix management errcheck
mcardosos Dec 9, 2016
f7e4e42
Exclude errors rule in gas scanning
mcardosos Dec 9, 2016
3fa71a0
Ignore checkerr issues involving Close()
mcardosos Dec 9, 2016
3b3aa5e
Get rid of gocheck, and add specific gas tests
mcardosos Dec 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ install:
- go get -u golang.org/x/net/context
- go get -u github.com/HewlettPackard/gas
- go get -u gopkg.in/godo.v2/cmd/godo
- go get -u github.com/kisielk/errcheck
- export GO15VENDOREXPERIMENT=1
- glide install

Expand All @@ -31,3 +32,10 @@ script:
- go vet ./management/...
- test -z "$(golint ./Gododir/... | tee /dev/stderr)"
- go vet ./Gododir/...
- errcheck -ignoretests ./arm/...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be shortened: set -e; for v in ./arm... ./datalake-store/... ./Gododir/...; do errcheck -ignoretests "$v"; done

- errcheck -ignoretests ./datalake-store/...
- errcheck -ignoretests ./Gododir/...
- errcheck -ignoretests ./management/...
- errcheck -ignoretests ./storage/...


14 changes: 10 additions & 4 deletions management/examples/example.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,19 @@ func main() {

fmt.Println("Create virtual machine")
role := vmutils.NewVMConfiguration(dnsName, vmSize)
vmutils.ConfigureDeploymentFromPlatformImage(
if err := vmutils.ConfigureDeploymentFromPlatformImage(
&role,
vmImage,
fmt.Sprintf("http://%s.blob.core.windows.net/%s/%s.vhd", storageAccount, dnsName, dnsName),
"")
vmutils.ConfigureForLinux(&role, dnsName, userName, userPassword)
vmutils.ConfigureWithPublicSSH(&role)
""); err != nil {
panic(err)
}
if err := vmutils.ConfigureForLinux(&role, dnsName, userName, userPassword); err != nil {
panic(err)
}
if err := vmutils.ConfigureWithPublicSSH(&role); err != nil {
panic(err)
}

fmt.Println("Deploy")
operationID, err := virtualmachine.NewClient(client).
Expand Down
6 changes: 4 additions & 2 deletions management/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import (
"net/http"
)

func getResponseBody(response *http.Response) ([]byte, error) {
defer response.Body.Close()
func getResponseBody(response *http.Response) (out []byte, err error) {
defer func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super ok with this one 1) response.Body can be nil 2) returning Close() error is a behavior change. also why do you add named returns?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm returning named results so the errors inside the defer function can actually be returned.
Question, will Close() return an error on an empty / nil response.Body? I that is the case then I agree it is not as useful to return an error from close, because ReadAll would just return an empty []byte.
Will fix

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question, will Close() return an error on an empty / nil response.Body?

it will panic with nil dereference error.

err = response.Body.Close()
}()
return ioutil.ReadAll(response.Body)
}
4 changes: 3 additions & 1 deletion management/vmutils/extensions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ func AddAzureDockerVMExtensionConfiguration(role *vm.Role, dockerPort int, versi
return fmt.Errorf(errParamNotSpecified, "role")
}

ConfigureWithExternalPort(role, "docker", dockerPort, dockerPort, vm.InputEndpointProtocolTCP)
if err := ConfigureWithExternalPort(role, "docker", dockerPort, dockerPort, vm.InputEndpointProtocolTCP); err != nil {
return err
}

publicConfiguration, err := createDockerPublicConfig(dockerPort)
if err != nil {
Expand Down
Loading