Issue 92610043: Add user infor to switch
DescriptionAdd user infor to switch
Append current user's username to current environment
info printed out by `$ juju switch`.
https://code.launchpad.net/~waigani/juju-core/switch-show-user/+merge/220868
(do not edit description out of merge proposal)
Patch Set 1 #
Total comments: 2
Patch Set 2 : Add user infor to switch #
Total comments: 2
Patch Set 3 : Add user infor to switch #
Total comments: 1
Patch Set 4 : Add user infor to switch #
Total comments: 3
Patch Set 5 : Add user infor to switch #Patch Set 6 : Add user infor to switch #
Total comments: 21
Patch Set 7 : Add user infor to switch #
Total comments: 18
Patch Set 8 : Add user infor to switch #
Total comments: 8
Patch Set 9 : Add user infor to switch #
MessagesTotal messages: 24
Please take a look.
Sign in to reply to this message.
LGTM. Nitpicks only. https://codereview.appspot.com/92610043/diff/1/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/1/cmd/juju/switch.go#newcode84 cmd/juju/switch.go:84: } This might be cleaner if you extracted the code to retrieve the current user to it's own function that printEnvInfo calls. Personally, I'd also move printEnvInfo underneath Run. It's considered good practice to have higher level functions towards the top of a source file and to have the lower level functions they use underneath. It makes the code easier for other developers to read because the lower level functionality can be easily ignored - or at least doesn't have to be digested up front - in order to get an understanding of what the code is doing and how it hangs together. https://codereview.appspot.com/92610043/diff/1/cmd/juju/switch.go#newcode153 cmd/juju/switch.go:153: } else { The 2 printEnvInfo calls here could be reduced to one by calling it after the "if currentEnv" block.
Sign in to reply to this message.
Please take a look.
Sign in to reply to this message.
https://codereview.appspot.com/92610043/diff/20001/cmd/juju/switch_test.go File cmd/juju/switch_test.go (right): https://codereview.appspot.com/92610043/diff/20001/cmd/juju/switch_test.go#ne... cmd/juju/switch_test.go:24: User: "joblow", Can we just use "joe" here? For reasons explained on IRC. https://codereview.appspot.com/92610043/diff/20001/cmd/juju/switch_test.go#ne... cmd/juju/switch_test.go:123: c.Assert(testing.Stdout(context), gc.Equals, "erewhemos -> erewhemos-2\nuser: "+testCreds.User+"\n") I think this would be better as: erewhemos -> erewhemos-2 as user "joe" Also, best to make the expected text explicit if you can.
Sign in to reply to this message.
Please take a look.
Sign in to reply to this message.
https://codereview.appspot.com/92610043/diff/40001/cmd/juju/switch_test.go File cmd/juju/switch_test.go (right): https://codereview.appspot.com/92610043/diff/40001/cmd/juju/switch_test.go#ne... cmd/juju/switch_test.go:65: c.Assert(testing.Stdout(context), gc.Equals, "erewhemos as user "+testCreds.User+"\n") As I mentioned before, make the string explicit: c.Assert(testing.Stdout(context), gc.Equals, `erewhemos as user "joe"` + "\n") Horrible having to add the + "\n" to the end, but better than ...\"joe\"\n"
Sign in to reply to this message.
On 2014/05/26 01:46:37, thumper wrote: > https://codereview.appspot.com/92610043/diff/40001/cmd/juju/switch_test.go > File cmd/juju/switch_test.go (right): > > https://codereview.appspot.com/92610043/diff/40001/cmd/juju/switch_test.go#ne... > cmd/juju/switch_test.go:65: c.Assert(testing.Stdout(context), gc.Equals, > "erewhemos as user "+testCreds.User+"\n") > As I mentioned before, make the string explicit: > > c.Assert(testing.Stdout(context), gc.Equals, `erewhemos as user "joe"` + "\n") > > Horrible having to add the + "\n" to the end, but better than ...\"joe\"\n" oops, missed that. Done now.
Sign in to reply to this message.
Please take a look.
Sign in to reply to this message.
https://codereview.appspot.com/92610043/diff/60001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/60001/cmd/juju/switch.go#newcode149 cmd/juju/switch.go:149: if username != "" { Hmm... is this implied "admin"? https://codereview.appspot.com/92610043/diff/60001/cmd/juju/switch_test.go File cmd/juju/switch_test.go (right): https://codereview.appspot.com/92610043/diff/60001/cmd/juju/switch_test.go#ne... cmd/juju/switch_test.go:65: c.Assert(testing.Stdout(context), gc.Equals, `erewhemos as user "`+testCreds.User+"\"\n") umm... your earlier comment said you fixed this. Did you commit it?
Sign in to reply to this message.
Please take a look.
Sign in to reply to this message.
https://codereview.appspot.com/92610043/diff/60001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/60001/cmd/juju/switch.go#newcode149 cmd/juju/switch.go:149: if username != "" { On 2014/05/26 04:09:42, thumper wrote: > Hmm... is this implied "admin"? If there is no .jenv file, username is not appended to output of switch. Are you saying that instead the "admin" user should be appended?
Sign in to reply to this message.
I'm not convinced by this. It's a compatibility break (which is not in itself necessarily bad -- we should be better when we can be) but (1) AIUI, people actively use this in scripts and (2) I'm not convinced it actually offers anything significantly better. * username is quoted but envname isn't * username sometimes shows up and sometimes doesn't * it's going to be kinda annoying to parse ISTM that if we're going to Do It Right we should collect a map (or even a struct) of all the relevant information and give it a --format arg (in which the default "smart" (lol) formatting can match the existing output for the next release. Counterarguments?
Sign in to reply to this message.
On 2014/05/26 07:26:56, fwereade wrote: > I'm not convinced by this. It's a compatibility break (which is not in itself > necessarily bad -- we should be better when we can be) but (1) AIUI, people > actively use this in scripts and (2) I'm not convinced it actually offers > anything significantly better. > > * username is quoted but envname isn't > * username sometimes shows up and sometimes doesn't > * it's going to be kinda annoying to parse > > ISTM that if we're going to Do It Right we should collect a map (or even a > struct) of all the relevant information and give it a --format arg (in which the > default "smart" (lol) formatting can match the existing output for the next > release. Counterarguments? *also*, this is *unquestionably* user-facing. We need docs for such features to exist at the time of writing, so that evilnick can see them and have a chance to massage them into a happy shape before the feature ends up in front of our users.
Sign in to reply to this message.
Please take a look.
Sign in to reply to this message.
Coming along nicely, thanks. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode53 cmd/juju/switch.go:53: $ juju switch --env-info Not *really* loving --env-info... "--explain" isn't great either, but it feels a bit closer to what we're asking for... or what about "--long"? we want to make it super-easy for people to use this, and get used to it -- for that matter, we should probably have a single-char flag too) https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode55 cmd/juju/switch.go:55: apiEndpoint: I think we need: environ-name [0] environ-uuid state-servers user-name ...but I'm not so sure ca-cert is very nice to include, because it'll be MASSIVE and push interesting stuff offscreen. [0] environ-name is strange and interesting, fwiw -- I think it can only ever really *sanely* be a local alias for a user/env pair -- but we *do* have environment names lurking in the state, meaningless though they may be. Can you think of a better way to describe what it actually is? https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode60 cmd/juju/switch.go:60: envName: local please use "names-with-hyphens", not namesWithCaps, for consistency. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode72 cmd/juju/switch.go:72: oldEnvName: ec2 previous-environ-name perhaps? https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode77 cmd/juju/switch.go:77: {"apiEndpoint": similarly, marshal to json with nice-names-like-this. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode83 cmd/juju/switch.go:83: "username":"joe" Thanks for the docs (for all that they're still in a state of flux) -- I find it *much* easier to think through the consistency and consequences and so on when the UX is laid out plainly like this. Please make sure evilnick knows about them so he has a chance to fix up the online docs for the next release -- we absolutely want his feedback on what's a good and easy-to-communicate experience.
Sign in to reply to this message.
Just some little things I noticed https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode76 cmd/juju/switch.go:76: # Format environment inforamtion as json information https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode190 cmd/juju/switch.go:190: } Can't the whole "if currentEnv" block now be replaced with: if err := c.printEnvInfo(ctx, c.EnvName, currentEnv); err != nil { return err } ? (The first case passes "" when currentEnv is "") https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode251 cmd/juju/switch.go:251: return username, nil Because this function uses named return values this line can just be "return" if you like.
Sign in to reply to this message.
Please take a look. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode53 cmd/juju/switch.go:53: $ juju switch --env-info On 2014/05/27 07:47:12, fwereade wrote: > Not *really* loving --env-info... "--explain" isn't great either, but it feels a > bit closer to what we're asking for... or what about "--long"? we want to make > it super-easy for people to use this, and get used to it -- for that matter, we > should probably have a single-char flag too) We already have single-char flag (-e), added example to docs. If we use "--long" we'd conflict with single-char flag for "--list", -li and/or -lo? https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode55 cmd/juju/switch.go:55: apiEndpoint: On 2014/05/27 07:47:12, fwereade wrote: > I think we need: > > environ-name [0] > environ-uuid > state-servers > user-name > > ...but I'm not so sure ca-cert is very nice to include, because it'll be MASSIVE > and push interesting stuff offscreen. > > [0] environ-name is strange and interesting, fwiw -- I think it can only ever > really *sanely* be a local alias for a user/env pair -- but we *do* have > environment names lurking in the state, meaningless though they may be. Can you > think of a better way to describe what it actually is? My understanding of environ-name is that it is simply the name of an environment, defined in environments.yaml. An environ must always have a user, but I don't see how that makes environ-name anything but the name of an environment still? Am I missing something? Can you give me an example of the problem? https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode60 cmd/juju/switch.go:60: envName: local On 2014/05/27 07:47:12, fwereade wrote: > please use "names-with-hyphens", not namesWithCaps, for consistency. Done. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode72 cmd/juju/switch.go:72: oldEnvName: ec2 On 2014/05/27 07:47:13, fwereade wrote: > previous-environ-name perhaps? Done. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode76 cmd/juju/switch.go:76: # Format environment inforamtion as json On 2014/05/27 21:16:05, menn0 wrote: > information Done. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode77 cmd/juju/switch.go:77: {"apiEndpoint": On 2014/05/27 07:47:12, fwereade wrote: > similarly, marshal to json with nice-names-like-this. Done. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode83 cmd/juju/switch.go:83: "username":"joe" On 2014/05/27 07:47:13, fwereade wrote: > Thanks for the docs (for all that they're still in a state of flux) -- I find it > *much* easier to think through the consistency and consequences and so on when > the UX is laid out plainly like this. > > Please make sure evilnick knows about them so he has a chance to fix up the > online docs for the next release -- we absolutely want his feedback on what's a > good and easy-to-communicate experience. Done. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode190 cmd/juju/switch.go:190: } On 2014/05/27 21:16:05, menn0 wrote: > Can't the whole "if currentEnv" block now be replaced with: > > if err := c.printEnvInfo(ctx, c.EnvName, currentEnv); err != nil { > return err > } > > ? > > (The first case passes "" when currentEnv is "") Nice spotting! Done. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode251 cmd/juju/switch.go:251: return username, nil On 2014/05/27 21:16:04, menn0 wrote: > Because this function uses named return values this line can just be "return" if > you like. This would return an 'is not found' error, when we don't want to.
Sign in to reply to this message.
https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode53 cmd/juju/switch.go:53: $ juju switch --env-info Why is this not just: juju switch --format=yaml ? https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode70 cmd/juju/switch.go:70: $ juju switch local --env-info Same here: juju switch local --format=yaml I don't think we need --env-info at all.
Sign in to reply to this message.
quick comments while I can, I'll try to do some more before you get online https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode55 cmd/juju/switch.go:55: apiEndpoint: On 2014/05/28 00:30:47, waigani wrote: > On 2014/05/27 07:47:12, fwereade wrote: > > I think we need: > > > > environ-name [0] > > environ-uuid > > state-servers > > user-name > > > > ...but I'm not so sure ca-cert is very nice to include, because it'll be > MASSIVE > > and push interesting stuff offscreen. > > > > [0] environ-name is strange and interesting, fwiw -- I think it can only ever > > really *sanely* be a local alias for a user/env pair -- but we *do* have > > environment names lurking in the state, meaningless though they may be. Can > you > > think of a better way to describe what it actually is? > > My understanding of environ-name is that it is simply the name of an > environment, defined in environments.yaml. An environ must always have a user, > but I don't see how that makes environ-name anything but the name of an > environment still? Am I missing something? Can you give me an example of the > problem? > I have an environment named "prod", and so do you. I give you access to mine. How do you disambiguate? Environment name was a really bad idea -- we made it something fundamental, instead of explicitly making it a local alias, and we continue to pay the price; but we're making some progress, because what you're actually choosing in the general case is now a *jenv* name, not an environment name. https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode251 cmd/juju/switch.go:251: return username, nil On 2014/05/28 00:30:47, waigani wrote: > On 2014/05/27 21:16:04, menn0 wrote: > > Because this function uses named return values this line can just be "return" > if > > you like. > > This would return an 'is not found' error, when we don't want to. And FWIW I have generally found that the benefits of empty returns are outweighed by the costs of (frequently) having to scan back through the function to be sure what actually happened. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode45 cmd/juju/switch.go:45: # Show current environment name General suggestion: # description $ command output output output ? https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode53 cmd/juju/switch.go:53: $ juju switch --env-info On 2014/05/28 02:26:46, thumper wrote: > Why is this not just: > juju switch --format=yaml > ? Very good point. We can define a "smart" (lol) formatter that preserves existing behaviour. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:197: servers, err := stateSevers(envName) s/Severs/Servers/ https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:205: "state-servers": servers, I'd be more inclined to define this as a struct, with explicit marshalling tags. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:216: if oldEnvName != "" { ...and then the version of "smart" formatter you'd define in here could just handle that particular struct according to the code here. (it's reasonable to argue that we shouldn't call it "smart" because that already defines a formatter that works differently; not sure I quite agree there but I'm not inclined to fight it if you feel that way. But then you'd have to come up with a name that I liked ;p)
Sign in to reply to this message.
https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/90001/cmd/juju/switch.go#newcode251 cmd/juju/switch.go:251: return username, nil On 2014/05/28 16:01:41, fwereade wrote: > On 2014/05/28 00:30:47, waigani wrote: > > On 2014/05/27 21:16:04, menn0 wrote: > > > Because this function uses named return values this line can just be > "return" > > if > > > you like. > > > > This would return an 'is not found' error, when we don't want to. > > And FWIW I have generally found that the benefits of empty returns are > outweighed by the costs of (frequently) having to scan back through the function > to be sure what actually happened. Yep you're right. This is probably a case of shiny-new-language-toy-itis on my part :) Sorry for the noise.
Sign in to reply to this message.
https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode53 cmd/juju/switch.go:53: $ juju switch --env-info On 2014/05/28 02:26:46, thumper wrote: > Why is this not just: > juju switch --format=yaml > ? +1 https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:205: "state-servers": servers, On 2014/05/28 16:01:41, fwereade wrote: > I'd be more inclined to define this as a struct, with explicit marshalling tags. +1 the contents of the output are important to users (e.g. scripts), so we should create a well-defined struct to deter people from making random changes to those contents. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:239: if err != nil { Why are we swallowing other errors?
Sign in to reply to this message.
Please take a look. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode45 cmd/juju/switch.go:45: # Show current environment name On 2014/05/28 16:01:41, fwereade wrote: > General suggestion: > > # description > $ command > output > output > output > > ? Done. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode53 cmd/juju/switch.go:53: $ juju switch --env-info On 2014/05/28 02:26:46, thumper wrote: > Why is this not just: > juju switch --format=yaml > ? Done. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode53 cmd/juju/switch.go:53: $ juju switch --env-info On 2014/05/28 02:26:46, thumper wrote: > Why is this not just: > juju switch --format=yaml > ? Done. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode53 cmd/juju/switch.go:53: $ juju switch --env-info On 2014/05/28 16:01:41, fwereade wrote: > On 2014/05/28 02:26:46, thumper wrote: > > Why is this not just: > > juju switch --format=yaml > > ? > > Very good point. We can define a "smart" (lol) formatter that preserves existing > behaviour. When formatter is "smart", no extra info is added and nothing is formatted. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcode70 cmd/juju/switch.go:70: $ juju switch local --env-info On 2014/05/28 02:26:46, thumper wrote: > Same here: > juju switch local --format=yaml > > I don't think we need --env-info at all. Done. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:197: servers, err := stateSevers(envName) On 2014/05/28 16:01:41, fwereade wrote: > s/Severs/Servers/ Done. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:205: "state-servers": servers, On 2014/05/28 16:01:41, fwereade wrote: > I'd be more inclined to define this as a struct, with explicit marshalling tags. Done. https://codereview.appspot.com/92610043/diff/110001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:239: if err != nil { On 2014/05/29 03:05:06, axw wrote: > Why are we swallowing other errors? To allow the user to switch to an environment that has not yet been bootstrapped.
Sign in to reply to this message.
Nearly there, I think it just needs another pass to rework printEnvInfo. I have a pretty strong feeling that that method should purely be about *constructing* an EnvInfo -- with all necessary information, specifically *not* tuning the information based on the formatter, which it shouldn't make use of -- and that the result-printing can then be handled, once, in Run (and as mentioned, the simple formatter can just ignore the fields it doesn't use). https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:119: c.EnvInfo = c.out.Name() != "simple" -1 on this. Can't we always grab the same information, and define a formatter that outputs the subset we want in the style we want? https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:266: return nil, nil return an actual error here please https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch_test.go File cmd/juju/switch_test.go (right): https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch_test.go#n... cmd/juju/switch_test.go:142: c.Assert(testing.Stdout(context), gc.Equals, It's rarely a good idea to test yaml directly -- better (if more tedious) to unmarshal whatever got marshalled and check that. (there are multiple valid forms of the yaml outout, and who knows what might cause a change in what actually gets written) https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch_test.go#n... cmd/juju/switch_test.go:154: c.Assert(testing.Stdout(context), gc.Equals, ditto https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch_test.go#n... cmd/juju/switch_test.go:166: c.Assert(testing.Stdout(context), gc.Equals, "{\"user-name\":\"joe\",\"environ-name\":\"erewhemos\",\"state-servers\":[\"example.com\",\"kremvax.ru\"]}\n") ditto https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch_test.go#n... cmd/juju/switch_test.go:173: c.Assert(testing.Stdout(context), gc.Equals, "{\"user-name\":\"joe\",\"environ-name\":\"erewhemos-2\",\"previous-environ-name\":\"erewhemos\",\"state-servers\":[\"example.com\",\"kremvax.ru\"]}\n") ditto
Sign in to reply to this message.
Please take a look. https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch.go File cmd/juju/switch.go (right): https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:119: c.EnvInfo = c.out.Name() != "simple" On 2014/06/02 08:56:03, fwereade wrote: > -1 on this. Can't we always grab the same information, and define a formatter > that outputs the subset we want in the style we want? Done. Actually, the current "simple" formatter already only prints out a subset. So I can simply remove c.EnvInfo. https://codereview.appspot.com/92610043/diff/130001/cmd/juju/switch.go#newcod... cmd/juju/switch.go:266: return nil, nil On 2014/06/02 08:56:03, fwereade wrote: > return an actual error here please Done.
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
