| === bigjools-afk is now known as bigjools | ||
| wrtp | fwereade: goat moaning | 07:37 |
|---|---|---|
| fwereade | wrtp, baaaaa | 07:37 |
| TheMue | mornin' | 08:27 |
| fwereade | heya TheMue | 08:28 |
| TheMue | fwereade: Today St Peter does his work to not make Roger jealous. It's raining here. | 08:29 |
| fwereade | TheMue, haha | 08:29 |
| wrtp | TheMue: yo! | 09:17 |
| wrtp | TheMue: it's dry here, at least | 09:17 |
| wrtp | TheMue: but still grey | 09:17 |
| wrtp | and cold | 09:17 |
| TheMue | wrtp: Temperature dropped by about 10° and rain has replaced sun. | 09:18 |
| fwereade | wrtp, good advice last night, I think it's much better: https://codereview.appspot.com/6163044/ | 09:24 |
| wrtp | fwereade: cool. yeah, i think that does look cleaner, thanks. | 09:26 |
| fwereade | wrtp, btw, I think it *is* a getCommand not a command -- but do you have a better name in mind? | 09:58 |
| fwereade | wrtp, maybe commands should be commandGetters | 09:58 |
| * wrtp has another look | 09:58 | |
| fwereade | wrtp, alittle voice from my misspent youth is telling me I should use the word "factory", but I'm trying to pretend I can't hear it ;) | 09:59 |
| wrtp | lol | 09:59 |
| wrtp | fwereade: perhaps the word "new" should come into it somewhere | 10:00 |
| fwereade | wrtp, not so sure, I feel that "new" binds too weakly | 10:00 |
| fwereade | wrtp, "newCommands" sounds to me like it's doing something diferent | 10:01 |
| wrtp | fwereade: newc ? cmdMaker ? plain "new" ? | 10:01 |
| wrtp | cf ? :-) | 10:01 |
| fwereade | wrtp, "constructors"? | 10:01 |
| wrtp | fwereade: i don't mind the name "commands". it's just "getCommand" that reads oddly to me. | 10:02 |
| wrtp | fwereade: it sounds more like it's getting a command that already exists rather than constructing a new one | 10:03 |
| fwereade | wrtp, a method called "GetCommand" that chooses a "getCommand" helper and returns its result feels pretty natural to me | 10:03 |
| fwereade | wrtp, hmm, yeah | 10:03 |
| wrtp | fwereade: or that it's an actual command that gets something | 10:03 |
| fwereade | wrtp, maybe the method itself should be NewCommand(string name) | 10:03 |
| wrtp | fwereade: +1 | 10:04 |
| fwereade | wrtp, then "newCommands" and "newCommand" feel good | 10:04 |
| fwereade | wrtp, cool | 10:04 |
| fwereade | thanks :) | 10:04 |
| wrtp | fwereade: tbh in such a local context, a short variable name would read well, i think. | 10:04 |
| wrtp | fwereade: then you have to look elsewhere for context, and you'll see the type of commands etc | 10:05 |
| fwereade | wrtp, "c" it is then :) | 10:05 |
| wrtp | fwereade: or "f"? given it's a function not a command. | 10:05 |
| fwereade | wrtp, +1 | 10:05 |
| * wrtp likes "NewCommand" | 10:06 | |
| wrtp | fwereade: LGTM | 10:18 |
| fwereade | wrtp, cheers | 10:18 |
| === TheMue_ is now known as TheMue | ||
| fwereade | wrtp, TheMue: should be utterly trivial: https://codereview.appspot.com/6184044 | 12:17 |
| TheMue | *click* | 12:18 |
| === asavu_ is now known as asavu | ||
| fwereade_ | morning niemeyer | 15:57 |
| niemeyer | fwereade_: Hey man! | 15:57 |
| niemeyer | fwereade_: Good timing, I was "talking to you" :-) | 15:57 |
| fwereade_ | niemeyer, dammit, is my astral projection wandering again? | 15:58 |
| fwereade_ | niemeyer, what can I do for you? | 15:58 |
| niemeyer | fwereade_: Was just answering https://codereview.appspot.com/6124051/ | 15:58 |
| * fwereade_ reads... | 15:59 | |
| fwereade_ | niemeyer, fair enough :) | 15:59 |
| fwereade_ | niemeyer, although I'm not sure how to communicate exit codes with that model | 16:00 |
| niemeyer | fwereade_: Hmm | 16:03 |
| niemeyer | fwereade_: I almost suggested before that the Main function in cmd should actually return an error instead, precisely for that reason | 16:04 |
| fwereade_ | niemeyer, I'd kinda prefer to keep the exit codes meaningful really | 16:05 |
| niemeyer | fwereade_: Maybe that's what we should do.. there's a single situation we return an error different than 1, right? | 16:05 |
| fwereade_ | niemeyer, Init errors return 2 | 16:06 |
| fwereade_ | niemeyer, which could really be caused by anything under the general heading of parsing | 16:06 |
| niemeyer | fwereade_: Ok.. anyway.. we can easily get going by merely returning the code from Main | 16:06 |
| fwereade_ | niemeyer, ok, sounds good | 16:07 |
| niemeyer | fwereade_: I've used both of these schemes in other applications before | 16:07 |
| niemeyer | fwereade_: Both the early dying and the return err | 16:07 |
| niemeyer | fwereade_: Returning errors generally ended up producing better code | 16:08 |
| fwereade_ | niemeyer, except, hmm again: a code of 1 could come either from the "real" command, or from jujuc, and I think we only want to print jujuc help in the latter case | 16:08 |
| niemeyer | fwereade_: Let's please never print help unless the user asks for it | 16:09 |
| fwereade_ | niemeyer, in practice I think that means either "never print help" or "complicate things an irritating amount to detect the cases in which the user asked for help and didn't get it from the real command" | 16:11 |
| niemeyer | fwereade_: I don't understand.. | 16:11 |
| niemeyer | fwereade_: If the user asked for help and didn't get from the real command, that's a bug in the real command | 16:12 |
| fwereade_ | niemeyer, I'm thinking of the case in which we couldn't talk to the agent for whatever reason | 16:12 |
| niemeyer | fwereade_: "error: couldn't talk to the agent"? | 16:12 |
| fwereade_ | niemeyer, that was the original intent: that the various things that can go wrong at this end print an appropriate error *and* an explanation of what's going on; otherwise everything is handled through the stuff returned by the agent | 16:13 |
| niemeyer | fwereade_: I understand, but I don't think explaining how things work every single time is necessary, or even nice | 16:15 |
| niemeyer | fwereade_: an error message explaining what went wrong should be good enough | 16:15 |
| niemeyer | fwereade_: ssh => invalid password; vs. ssh => invalid password + ssh is an application that requires a password to be set blah blah blah | 16:16 |
| fwereade_ | niemeyer: so I should change this everywhere? | 16:18 |
| niemeyer | fwereade_: Do we have other locations doing the same thing? | 16:18 |
| niemeyer | fwereade_: I recall making the same comment elsewhere before | 16:18 |
| fwereade_ | niemeyer, parse errors print help; others don't | 16:18 |
| niemeyer | fwereade_: I'm personally less concerned about those, but I don't mind if you decide to change them to behave that way as wel | 16:19 |
| niemeyer | l | 16:19 |
| fwereade_ | niemeyer, jujuc seemed to me to be a special case in that an error out of jujuc itself implies extreme brokenness | 16:19 |
| fwereade_ | niemeyer, personally I like getting help when I type utter gibberish | 16:20 |
| fwereade_ | niemeyer, but that's by the by | 16:21 |
| niemeyer | fwereade_: Sure, if you want to keep showing help on parse errors, I personally don't mind either. That said, I don't *miss* the help, even in those cases | 16:21 |
| fwereade_ | niemeyer, assumed context for printing help when jujuc is run in a bad environment is "users should only actually see this when they try to run jujuc on its own" | 16:21 |
| niemeyer | fwereade_: I prefer a one-liner error saying "error: you mistyped --gibberush" than "blah blah blah blah blah blah blah.. and you mistyped --gibberush" | 16:22 |
| niemeyer | fwereade_: "error: jujuc must be called from its symlinks"? | 16:23 |
| fwereade_ | niemeyer, hmm, ok, how about printing that iff filepath.Base(os.Args[0]) == "jujuc" | 16:24 |
| fwereade_ | niemeyer, and otherwise not offering help (except as supplied by remote commands ofc) | 16:25 |
| niemeyer | fwereade_: Sounds good | 16:25 |
| fwereade_ | niemeyer, cool, thanks | 16:25 |
| niemeyer | fwereade_: Thank you, and sorry for not being very clear about those ideas before | 16:29 |
| wrtp | niemeyer: hiya | 16:36 |
| wrtp | niemeyer: any chance of a review of https://codereview.appspot.com/6159044/ ? | 16:36 |
| niemeyer | wrtp: Ah, sorry.. I half-reviewed it already but forgot to finish yesterday | 16:37 |
| wrtp | niemeyer: np. i'm working on another s3 branch which i'd hoped would be independent but turns out it's not. | 16:37 |
| niemeyer | wrtp: LGTM | 16:38 |
| wrtp | niemeyer: cool, thanks. | 16:38 |
| * TheMue will next propose the relation methods of topology. The code is even more compat than Py. Nice. | 16:39 | |
| niemeyer | TheMue: Sweet | 16:40 |
| TheMue | niemeyer: Yes, indeed. But I still would like you to tell me a bit more about the relations concept next week. Inside topology it's simple, but I'm still feeling unsecure regarding the relation manager etc. | 16:41 |
| niemeyer | TheMue: SOunds good | 16:45 |
| TheMue | niemeyer: Thx | 16:45 |
| jimbaker | hazmat, were you able to take a look at lp:~jimbaker/juju/unit-rel-lifecycle-start-invoker ? (https://codereview.appspot.com/6131061/) | 16:51 |
| hazmat | jimbaker, checking it now | 16:52 |
| jimbaker | hazmat, thanks | 16:52 |
| fwereade_ | gn all | 16:52 |
| wrtp | fwereade_: see ya tomorrowq | 16:53 |
| wrtp | niemeyer: just discovered a slight oddity in the s3 package | 16:53 |
| wrtp | niemeyer: is it right that the ETag has double-quotes around it? | 16:53 |
| wrtp | niemeyer: the tests check that, but it seems odd | 16:54 |
| niemeyer | wrtp: I don't really have memories about that anymore, but if it's there, it means I've probably seen somewhere at least | 16:59 |
| wrtp | niemeyer: yeah, it seems weird, but i'll add a bit of documentation for others to note. | 16:59 |
| niemeyer | wrtp: Worth testing | 17:00 |
| wrtp | niemeyer: yeah, it bowled out an error in my test server. | 17:00 |
| TheMue | Hmm, today my net is pretty unstable. | 17:05 |
| wrtp | niemeyer: https://codereview.appspot.com/6185044 | 17:17 |
| wrtp | niemeyer: sorry about the gofmt noise, but i think it's easy to see when a file has only changed white space | 17:18 |
| wrtp | right, good moment to stop. | 17:24 |
| wrtp | see y'all tomorrow | 17:24 |
| niemeyer | wrtp: Cool | 17:29 |
| niemeyer | wrtp: have a good time | 17:29 |
| hazmat | jimbaker, sorry got de-railed for scale test demo of juju on 10k nodes.. | 17:42 |
| jimbaker | hazmat, no worries | 17:54 |
| fwereade_ | niemeyer, if you have a moment, I'm not sure what the issue is with contexts | 18:29 |
| niemeyer | fwereade_: I'm not sure either, so happy to talk if you're not sleeping yet :) | 18:30 |
| fwereade_ | niemeyer, there is server.Context and cmd.Context which are entirely distinct | 18:30 |
| niemeyer | fwereade_: The basic question is: how come we have two contexts, and one of the is completely ignored? | 18:30 |
| niemeyer | fwereade_: Yes, and that's confusing.. why there are two contexts and why is one of them getting into the function without being used? | 18:31 |
| fwereade_ | niemeyer, the cmd.Context isn't used if we happen not to use stdout/stderr/working-dir | 18:31 |
| niemeyer | fwereade_: It's quite possible that it's totally fine, but the fact I'm confused is at least an indicator that it'll not be obvious why that exists in a bit | 18:31 |
| fwereade_ | niemeyer, I think/hope that the source of the confusion is the fact that we have 2 different typs in different packages whih have the same name and end up being used close to one another | 18:32 |
| niemeyer | fwereade_: It's more than that.. we have two different types, in different types, used for the same thing, and getting to the context in two different ways, with one of them being entirely ignored at times | 18:33 |
| niemeyer | s/in different types/in different packages/ | 18:33 |
| fwereade_ | niemeyer, how are they being used for the same thing? | 18:33 |
| fwereade_ | niemeyer, one of them is about th ee | 18:33 |
| niemeyer | fwereade_: Maybe I'm just confused then | 18:33 |
| * niemeyer looks at the code | 18:33 | |
| fwereade_ | niemeyer, ...the execution environment, and one of them is about the state we're working against | 18:33 |
| fwereade_ | niemeyer, I haven't found it confusing myself *but* that's no doubt because I'm in an extremely privileged position there | 18:35 |
| fwereade_ | niemeyer, the name "Context", now I think of it, is not much better than "DataManager" | 18:36 |
| niemeyer | fwereade_: DataManager is as good as Struct :-) | 18:36 |
| fwereade_ | niemeyer, I had hoped the cmd.Context/server.Context distinction would be considered appropriately gotastic, but clearly that's not the case ;) | 18:36 |
| niemeyer | fwereade_: and the issue may come out of the naming indeed.. by definition, a thing executing in two contexts at the same time is unexpected | 18:37 |
| fwereade_ | niemeyer, part of me wanted to call server.Context something like server.State, but then we hit the same issue with state.State | 18:37 |
| niemeyer | fwereade_: Their name is ok in isolation, but the moment you have two contexts at once, something feels wrong | 18:38 |
| niemeyer | fwereade_: The issue is partly in my head indeed, anyway | 18:39 |
| niemeyer | fwereade_: Don't worry about it.. the issue is less important than I though | 18:41 |
| niemeyer | fwereade_: We can find a better name later if necessary | 18:41 |
| niemeyer | fwereade_: I'll review the code again in light of your explanation, thanks | 18:41 |
| fwereade_ | niemeyer, cool, thanks -- I was toying with explicit cmdCtx/serverCtx throughout, but that felt painful too | 18:41 |
| fwereade_ | niemeyer, suggestions appreciated :) | 18:42 |
| niemeyer | fwereade_: I don't have any good suggestions either right now.. if I come up with something that you might like I'll post in the review | 18:42 |
| niemeyer | fwereade_: Thanks, and sorry for the noise | 18:42 |
| fwereade_ | niemeyer, cheers | 18:42 |
| fwereade_ | niemeyer, and no worries :) | 18:42 |
| fwereade_ | ttyl | 18:43 |
| niemeyer | fwereade_: Ok, got a more concrete suggestion that might take away the minor confusion | 19:05 |
| niemeyer | fwereade_: Review is up, with an associated LGTM assuming you're happy with it too | 19:05 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!