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

Issue 90740044: juju/api.go: actually handle error (bug #1312136)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years ago by natefinch
Modified:
10 years ago
Reviewers:
wwitzel3, mp+217084, fwereade
Visibility:
Public.

Description

juju/api.go: actually handle error (bug #1312136) (refactored from a change John made) If there is a problem reading the .jenv file, it can return an error, but we weren't paying attention to it. Instead we would end up getting a panic() a few lines down when we go to use a *Config that is actually nil. addition by Nate: Refactored the code to extract the logic for getting the config into a separate function, which cleans up the error handling significantly. https://code.launchpad.net/~natefinch/juju-core/jameinel-1.18-panic-parsing-jenv-1312136/+merge/217084 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : juju/api.go: actually handle error (bug #1312136) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -18 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M juju/api.go View 1 3 chunks +26 lines, -16 lines 0 comments Download
M juju/apiconn_test.go View 1 chunk +20 lines, -0 lines 0 comments Download
M juju/export_test.go View 1 chunk +1 line, -0 lines 0 comments Download
M scripts/win-installer/setup.iss View 1 chunk +1 line, -1 line 0 comments Download
M version/version.go View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4
natefinch
Please take a look.
10 years ago (2014-04-24 16:17:27 UTC) #1
natefinch
Please take a look.
10 years ago (2014-04-24 16:27:35 UTC) #2
fwereade
LGTM
10 years ago (2014-04-24 16:38:35 UTC) #3
wwitzel3
10 years ago (2014-04-24 17:07:53 UTC) #4
LGTM

Nice refactor. I like this much better.
Sign in to reply to this message.

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