Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(130)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 8 months ago by natefinch
Modified:
11 years, 8 months ago
Reviewers:
wwitzel3 , mp+217084, fwereade
Visibility:
Public.
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) #

Created: 11 years, 8 months ago
Download [raw] [tar.bz2]
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
Total messages: 4
|
natefinch
Please take a look.
11 years, 8 months ago (2014年04月24日 16:17:27 UTC) #1
Please take a look.
Sign in to reply to this message.
natefinch
Please take a look.
11 years, 8 months ago (2014年04月24日 16:27:35 UTC) #2
Please take a look.
Sign in to reply to this message.
fwereade
LGTM
11 years, 8 months ago (2014年04月24日 16:38:35 UTC) #3
LGTM
Sign in to reply to this message.
wwitzel3
LGTM Nice refactor. I like this much better.
11 years, 8 months 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

AltStyle によって変換されたページ (->オリジナル) /