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

Issue 12546043: return error on no environment

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 9 months ago by natefinch
Modified:
8 years, 9 months ago
Reviewers:
dimitern, mp+178812, axw1, jameinel, rog
Visibility:
Public.

Description

return error on no environment Now, instead of just returning a cryptic filenotfound error, we return a more specific NoEnvError, indicating the environment doesn't exist. When the commands encounter that error, they notify the user, who is told how to create a new environment. https://code.launchpad.net/~natefinch/juju-core/001-noenv-warning/+merge/178812 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 8

Patch Set 2 : return error on no environment #

Total comments: 25

Patch Set 3 : return error on no environment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+219 lines, -156 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/addrelation.go View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M cmd/juju/bootstrap.go View 1 2 3 chunks +2 lines, -11 lines 0 comments Download
M cmd/juju/bootstrap_test.go View 1 2 chunks +0 lines, -12 lines 0 comments Download
M cmd/juju/config_test.go View 1 2 5 chunks +21 lines, -20 lines 0 comments Download
M cmd/juju/destroyrelation.go View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M cmd/juju/destroyunit.go View 1 2 1 chunk +1 line, -1 line 0 comments Download
M cmd/juju/environment.go View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M cmd/juju/environmentcommand.go View 1 2 chunks +6 lines, -0 lines 0 comments Download
M cmd/juju/expose.go View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M cmd/juju/get.go View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M cmd/juju/init.go View 1 2 chunks +3 lines, -2 lines 0 comments Download
M cmd/juju/init_test.go View 1 2 2 chunks +20 lines, -18 lines 0 comments Download
M cmd/juju/main.go View 1 2 2 chunks +74 lines, -36 lines 0 comments Download
M cmd/juju/publish.go View 1 chunk +5 lines, -3 lines 0 comments Download
M cmd/juju/set.go View 1 1 chunk +1 line, -0 lines 0 comments Download
M cmd/juju/unexpose.go View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M cmd/juju/upgradejuju.go View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M environs/config.go View 1 2 2 chunks +14 lines, -0 lines 0 comments Download
M environs/config_test.go View 1 2 10 chunks +60 lines, -52 lines 0 comments Download

Messages

Total messages: 14
natefinch
Please take a look.
8 years, 9 months ago (2013-08-06 17:05:42 UTC) #1
axw1
On 2013/08/06 17:05:42, nate.finch wrote: > Please take a look. LGTM, but I do wonder ...
8 years, 9 months ago (2013-08-07 02:06:34 UTC) #2
axw1
https://codereview.appspot.com/12546043/diff/1/cmd/juju/environmentcommand.go File cmd/juju/environmentcommand.go (right): https://codereview.appspot.com/12546043/diff/1/cmd/juju/environmentcommand.go#newcode75 cmd/juju/environmentcommand.go:75: // the caller was using the Missing the end ...
8 years, 9 months ago (2013-08-07 02:07:23 UTC) #3
dimitern
Nice! LGTM with a few suggestions below https://codereview.appspot.com/12546043/diff/1/cmd/juju/environmentcommand.go File cmd/juju/environmentcommand.go (right): https://codereview.appspot.com/12546043/diff/1/cmd/juju/environmentcommand.go#newcode75 cmd/juju/environmentcommand.go:75: // the ...
8 years, 9 months ago (2013-08-07 09:37:29 UTC) #4
rog
I think nate has a good point actually, and we can avoid all those envOpenFailure ...
8 years, 9 months ago (2013-08-07 15:45:57 UTC) #5
rog
On 2013/08/07 15:45:57, rog wrote: > I think nate has a good point actually, and ...
8 years, 9 months ago (2013-08-07 15:47:04 UTC) #6
natefinch
> I think [Andrew] has a good point actually, and we > can avoid all ...
8 years, 9 months ago (2013-08-07 20:25:53 UTC) #7
natefinch
Please take a look.
8 years, 9 months ago (2013-08-08 16:53:38 UTC) #8
dimitern
LGTM, if you revert the changes to files which do not need Run(ctx ..) and ...
8 years, 9 months ago (2013-08-08 17:16:52 UTC) #9
rog
LGTM modulo trivials below. https://codereview.appspot.com/12546043/diff/13001/cmd/juju/addmachine.go File cmd/juju/addmachine.go (right): https://codereview.appspot.com/12546043/diff/13001/cmd/juju/addmachine.go#newcode70 cmd/juju/addmachine.go:70: func (c *AddMachineCommand) Run(ctx *cmd.Context) ...
8 years, 9 months ago (2013-08-08 17:25:40 UTC) #10
natefinch
Please take a look.
8 years, 9 months ago (2013-08-08 20:37:47 UTC) #11
natefinch
I did everything as asked, except for Roger's change to main.go... I intentionally am keeping ...
8 years, 9 months ago (2013-08-08 20:39:42 UTC) #12
jameinel
what is this still waiting on? Is it just that Nate wants people to confirm ...
8 years, 9 months ago (2013-08-13 13:22:35 UTC) #13
jameinel
8 years, 9 months ago (2013-08-13 13:56:34 UTC) #14
On 2013/08/13 13:22:35, jameinel wrote:
> what is this still waiting on? Is it just that Nate wants people to confirm
that
> he did the changes they requested?

The reviews were "LGTM with these changes" which generally means that if you
make those changes, you can mark the request Approved and land it.
You only need to seek further reviews if you feel you didn't understand/didn't
quite do what was requested, and you want to continue the conversation.

So I think you can just land this, but I didn't do an actual review myself.
Sign in to reply to this message.

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