[07:46] <wrtp> fwereade: morning!
[07:47] <fwereade> wrtp, heyhey
[07:47] <fwereade> wrtp, how are you?
[07:47] <wrtp> fwereade: just got back from my bike ride; not raining this morning, yay!
[07:48] <fwereade> wrtp, nice
[07:48] <wrtp> fwereade: and my 82 year old ex-lorry-driver neighbour just told me that 12.04 is out on Thursday
[07:48] <fwereade> wrtp, awesome :D
[07:48] <wrtp> fwereade: (which i didn't actually know!)
[07:49] <fwereade> wrtp, better yet -- community in action ;p
[08:25] <TheMue> morning
[08:35] <fwereade> heya TheMue
[10:53] <TheMue> Lunchtime ...
[11:10] <wrtp> fwereade: i've been thinking about SuperCommand etc
[11:10] <fwereade> wrtp, cool, go on
[11:11] <wrtp> fwereade: the fact that when SuperCommand parses itself again it misses the flags added by LoggingCommand seems like an indication of something wrong structurally.
[11:12] <wrtp> fwereade: and i *think* i have an idea for a way to fix it
[11:12] <fwereade> wrtp, cool, it's seemed likely that everyone else can see an obvious design that I've missed ;P
[11:13] <wrtp> fwereade: the underlying problem is, i think, that you're trying to use LoggingCommand to add functionality to SuperCommand, but it doesn't - it just embeds it as you know.
[11:13] <fwereade> wrtp, it seemed to me like a good and sane consequence of embedding over inheritance
[11:13] <fwereade> wrtp, yep
[11:13] <wrtp> fwereade: how about actually providing a hook for SuperCommand to do the flag parsing.
[11:13] <wrtp> fwereade: because SuperCommand doesn't have any flags itself, so why not let its user add them?
[11:14]  * fwereade thinks
[11:14] <wrtp> fwereade: so it could have a SetInitFlagSet(func(*gnuflag.FlagSet)) method for example
[11:17] <fwereade> wrtp, I think it'd also need hooks for the other methods too though
[11:17] <wrtp> fwereade: or that function could be an argument to NewSuperCommand
[11:17] <fwereade> wrtp, we need to do something with them in Run()
[11:17] <wrtp> fwereade: which other methods?
[11:18] <fwereade> wrtp, Run and potentially ParsePositional-or-whatever-it-is could each want to do something with the flags on the logging type
[11:19] <wrtp> fwereade: ParsePositional sounds wrong, as it would never get any positional arguments
[11:19] <fwereade> wrtp, ParsePositional on supercommand always gets the subcommand as a positional arg
[11:20] <fwereade> wrtp, and all the subcmd's args as well because it doesn't intersperse
[11:21] <fwereade> wrtp, (incidentally I like `func Reduce(args []string) (Command, []string, error)`)
[11:21] <fwereade> wrtp, then for consistency's sake you'd want (bare) Parse to return the final command ready to Run
[11:22] <fwereade> wrtp, and that would actually always be the one originally passed in, but doesn't *have* to be
[11:24] <wrtp> oh the twisted heaps we create just so we can intermix global flags and command-specific flags
[11:27] <wrtp> fwereade: the thing is that having Reduce return a Command is *only* useful for the supercommand case AFAICS
[11:28] <fwereade> wrtp, sure, but that bothers the other commands not one bit
[11:29] <wrtp> fwereade: so rather than make that interface, which is used all over the place, more complex, why don't we make SuperCommand (which is a very small amount of code currently) slightly more complex instead?
[11:29] <fwereade> wrtp, and it has a nice fit with the actual problem it's solving -- it is intuitively sensible that the process of parsing a given command be identical whether or not it's actually a subcommand
[11:30] <fwereade> wrtp, it's used a certain amount; it amounts to writing ", nil" in a very few places
[11:30] <wrtp> fwereade: i'm looking at the code that was deleted from SuperCommand in this CL and thinking: why can't the Logging piece implement just that code
[11:31] <wrtp> fwereade: if you have that interface, it implies that a subcommand can itself return a subcommand
[11:31] <fwereade> wrtp, IMO this is less of a cognitive load than adding two separate hooks to SuperCommand for InitFlagSet and Run
[11:31] <fwereade> wrtp, well, actually, is there any reason it shouldn't?
[11:32] <wrtp> fwereade: well, your current code wouldn't work - you'd need a loop
[11:32] <fwereade> wrtp, like I suggest in the discussion already?
[11:32] <wrtp> fwereade: oh, i didn't see that!
[11:32] <fwereade> wrtp, ok it's a rubbish loop as I discovered when checking against reality but the core idea is there
[11:33] <wrtp> fwereade: ok, here's another idea: make NewSuperCommand take a Command as an argument
[11:33] <wrtp> fwereade: it will call Run but always with zero arguments
[11:34] <fwereade> wrtp, hmm, at least it concatenates all the hooks into one package ;p
[11:35] <fwereade> wrtp, I'm not sure that's intrinsically *better* than the loop idea... the fact that the loop works however many subcommands you have implies to me that it's well-suited to the problem
[11:35] <wrtp> fwereade: then you'd call NewSuperCommand(name, doc, &LoggingSetupCommand{})
[11:35] <wrtp> fwereade: to me it seems like overengineering. that's not the problem we're trying to solve.
[11:35] <fwereade> wrtp, yeah, but the set of Commands that can usefully go in there is limited
[11:36] <fwereade> wrtp, it's not the same thing as a Command, its fundamental nature is a set of hooks rather than a command
[11:37] <wrtp> fwereade: true. alternative: SuperCommand could define its own interface type to be used as an argument.
[11:37] <fwereade> wrtp, this is still fundamentally the hooks idea
[11:38] <wrtp> fwereade: indeed. but i think that's fundamentally what's going on. you want to "hook" the global flags into any arbitrary subcommand
[11:38] <fwereade> wrtp, and given that we have just one way in which we need to tweak SuperCommand
[11:38] <fwereade> wrtp, we end up reducing the code to precisely the original implementation
[11:39] <fwereade> wrtp, because SuperCommand checks some status and then does something in response in each of InitFlagSet and Run
[11:39] <fwereade> wrtp, and it's just that it's an implicit do-we-have-a-hook check instead of an explicit self.SetsLog check
[11:39] <fwereade> wrtp, with additional tomfoolery to set it all up
[11:40] <fwereade> wrtp, I don;t feel that's a win
[11:40] <wrtp> fwereade: i'm not sure
[11:41] <fwereade> wrtp, it may be more extensible in the future, but so is mine; they're both interestingly extensible in different ways and I don't think we currently have reason to prefer one over the other on that front
[11:41] <wrtp> fwereade: i think the hooks possibility makes it easy to add more global flags that aren't related to logging.
[11:41] <fwereade> wrtp, sure, that would be an awesome change at precisely the moment we actually have such a need
[11:41] <wrtp> fwereade: and i think that the "return a command" thing we will never ever use apart from in supercommand. it seems to me like we're spreading supercommand mess around.
[11:43] <wrtp> fwereade: for the record, this is the kind of thing i'm thinking of: http://paste.ubuntu.com/943945/
[11:44] <wrtp> fwereade: then all the ParsePositional stuff can remain exactly as it is now.
[11:45] <wrtp> fwereade: and the logging set up code is almost identical to now
[11:46] <fwereade> wrtp, ok, so that's still an extra type and extra setup work to do everything the original implementation did, in the same way, which I was asked to change
[11:46] <wrtp> fwereade: that's true. but at least the concerns are now separated. SuperCommand now has nothing to do with logging. and the logic is quite easy to follow, i think. (but then i would, wouldn't i? :-))
[11:47] <fwereade> wrtp, how does SuperCommand as separated out have anything to do with logging in the first place?
[11:48] <wrtp> fwereade: it doesn't, but it did, didn't it?
[11:48] <wrtp> sorry, i'm not sure what the question is
[11:48] <wrtp> fwereade: SuperCommand as you're proposing has nothing to do with logging, no.
[11:49] <wrtp> fwereade: but i'm not keen on the way that it makes all commands into recursive Command producers. i think that adds to cognitive load, when we can isolate it into SuperCommand.
[11:50] <fwereade> wrtp, I think that the right doc for Reduce could make it all very clear
[11:51] <fwereade> wrtp, in practice, returning 2 nils in a few places is not an especially painful cost
[11:51] <wrtp> fwereade: but there's always the question: why are *all* these commands returning Command when *none* of them apart from SuperCommand uses that functionality?
[11:51] <wrtp> fwereade: or will ever use that functionality...
[11:52] <fwereade> wrtp, can we keep the unknowable future off the table?
[11:54] <wrtp> fwereade: when i first read the SuperCommand code, i had to spend a little while, as i didn't find it easy to see what was going on. I now understand it (and I think it's nice for the problem being solved), but i really think that the whole recursive command thing belongs *there*, not in every command that's being implemented. the Command interface should map nicely to the command interface we want to implement IMHO.
[11:58]  * fwereade is still thinking
[11:59]  * wrtp is trying to work out if he's being unreasonable.
[12:00]  * wrtp hopes not :-)
[12:00] <fwereade> wrtp, I understand your concerns -- I think -- but they don't carry the same weight in my mind
[12:00] <fwereade> rogpeppe, ^^
[12:01] <rogpeppe> :-)
[12:01] <fwereade> rogpeppe, consider that (bare) Parse itself is only ever called with a SuperCommand in my proposal
[12:01] <fwereade> rogpeppe, there is, as there always was, a certain commingling of functionality
[12:02] <rogpeppe> fwereade: which Parse are you referring to there?
[12:02] <fwereade> rogpeppe, the precise balance at a given point depends on the needs of the current codebase
[12:02] <fwereade> rogpeppe, cmd.Parse
[12:04] <fwereade> rogpeppe, it will surely become immediately apparent if the future needs of the codebase require that we rearrange the responsibilities of the cmd package
[12:04] <rogpeppe> [13:01] <fwereade> rogpeppe, consider that (bare) Parse itself is only ever called with a SuperCommand in my proposal
[12:04] <rogpeppe> isn't it called with a LoggingSuperCommand?
[12:05] <fwereade> rogpeppe, ha, ok, something that flawlessly impersonates a SuperCommand, if you must
[12:06] <fwereade> rogpeppe, but that definition only lends weight to my assertion that Parse, which is called with two distinct things that both expect this behaviour, is correct to accommodate their shared functionality
[12:06] <rogpeppe> fwereade: not quite flawlessly... (given the recursive calling problem) :-)
[12:06]  * fwereade concedes somewhat grudgingly
[12:08] <fwereade> rogpeppe, the trouble is still that, considering proposals [rog] and [original], [rog] does what [original] did, and adds a bunch of other stuff that's only used in one case; [original] failed to find favour, and I don't think that an implementation that does just the same thing, but with more boilerplate, is likely to fly
[12:08] <niemeyer> Hello!
[12:08] <fwereade> niemeyer, heyhey
[12:08] <rogpeppe> niemeyer: yo!
[12:09] <niemeyer> fwereade: While you're there, I don't understand why we need recursion at all :-)
[12:09] <rogpeppe> +1 !
[12:09] <fwereade> niemeyer, rogpeppe: it is now a loop ;p
[12:09] <niemeyer> fwereade: Not even that!
[12:10] <rogpeppe> fwereade: recursion via Y-combinator :-)
[12:10] <niemeyer> fwereade: more, err := scmd.Consume(args)
[12:10] <niemeyer> fwereade: ...
[12:10] <fwereade> niemeyer: no, we need some sort of repeated parsing whatever we do
[12:10] <niemeyer> fwereade: subcmd := scmd.subcmds[name]
[12:10] <niemeyer> fwereade: subcmd.Consume(args)
[12:10] <rogpeppe> niemeyer: this is my current preferred proposal: http://paste.ubuntu.com/943945/
[12:10] <niemeyer> fwereade: This is a linear call.. supercommand calls subcommand..
[12:11] <TheMue> niemeyer: moin
[12:11]  * niemeyer looks
[12:11] <niemeyer> TheMue: Heya
[12:11] <fwereade> niemeyer, wait, so supercommand Consume calls subcmd Consume?
[12:11] <niemeyer> rogpeppe: That works too
[12:12] <niemeyer> fwereade: Yeah, that sounds fine in principle.. am I missing something?
[12:12] <fwereade> niemeyer, well, subcommands can have flags, and cmd.Parse is already set up to deal with flags before passing the positional args on to Reduce/Consume/whatever-it-currently-is
[12:13] <fwereade> niemeyer, duplicating that in SuperCommand would seem to be kinda rubbish
[12:13] <rogpeppe> i like the current names. InitFlagSet, ParsePositional and Run work well for me.
[12:14] <fwereade> niemeyer, rogpeppe: we have always had recursive Parse calls; they were disguised by being indirect, via ParsePositional, but it was always how it worked
[12:14] <niemeyer> fwereade: Sorry, I don't understand.. probably because I don't have the picture so clear in my head as you do
[12:14] <rogpeppe> as i said earlier, that's an interface that works well for commands, even if it isn't quite sufficient for SuperCommand.
[12:14] <niemeyer> fwereade: I know.. and as I mentioned in the review, I've always disliked that.. it's magical, and unnecessary, iMO
[12:14] <niemeyer> fwereade: The problem seems so simple.. I don't get why we need all this back and forth
[12:15] <niemeyer> fwereade: SuperCommand wraps a number of subcommands.. it gets called, handles what it must, and delegates the rest
[12:15] <niemeyer> fwereade: No recursion, no looping
[12:15] <rogpeppe> niemeyer: the fundamental difficulty is the mixing of global and sumcommand flags AFAICS
[12:15] <rogpeppe> s/sumcom/subcom/
[12:15] <fwereade> niemeyer, how many times do we call Parse on a FlagSet in the course of this?
[12:15] <fwereade> niemeyer, twice! there are two parsing steps needed
[12:16] <rogpeppe> and we call InitFlagSet three times, right?
[12:16] <niemeyer> fwereade: Yep.. twice, exactly.. not more, not less
[12:16] <fwereade> rogpeppe, we may create a fresh FlagSet for usage output
[12:16] <niemeyer> fwereade: No need for recursion or looping in that case
[12:16] <rogpeppe> twice on the supercommand and once on the subcommand
[12:16] <rogpeppe> fwereade: ah, makes sense
[12:17] <fwereade> niemeyer, well, once *or* twice really, given that we expect Parse to work on anything that implements Command
[12:17] <niemeyer> fwereade: Parse should be completely unaware about any of that
[12:18] <rogpeppe> fwereade: is there a particular reason we export cmd.Parse BTW? does anything outside of the cmd package actually use it?
[12:18] <niemeyer> fwereade: as should the interface of Command
[12:18] <niemeyer> fwereade: The only particularity is that a supercommand needs to call the subcommand.. that's all really
[12:18] <niemeyer> fwereade: The mechanisms out of the supercommand should not be aware of that
[12:18] <fwereade> niemeyer, and mingle flaget flags
[12:19] <rogpeppe> i'm all for mingling flagets
[12:19] <niemeyer> fwereade: ?
[12:20] <fwereade> niemeyer, the client of Parse doesn't see a difference; AFAICT the point of difficulty is in the interface for Command itself, right?
[12:21] <fwereade> niemeyer, sorry, misunderstood
[12:21] <niemeyer> fwereade: Maybe I should just try to code a sketch
[12:21] <rogpeppe> fwereade: that's true for me, certainly. i think the interface for Command works well, and we're mangling it solely because of SuperCommand issues.
[12:21] <fwereade> niemeyer, I was saying the SC needs both to select subcommands and reregister its own flags
[12:21] <niemeyer> fwereade: I'm unhappy about this:
[12:21] <niemeyer> func (c *SuperCommand) InitFlagSet(f *gnuflag.FlagSet) {
[12:21] <niemeyer>         if c.subcmd != nil {
[12:21] <niemeyer>                 c.subcmd.InitFlagSet(f)
[12:22] <niemeyer> fwereade: This is all magical..
[12:22] <niemeyer> fwereade: It's called once, somewhere, and then twice, in a full moon
[12:22] <niemeyer> fwereade: Maybe it's set, or not.. and behaves in one way or the other..
[12:23] <niemeyer> fwereade: and most methods go like that..
[12:23] <niemeyer> fwereade:
[12:23] <niemeyer> func (c *SuperCommand) Info() *Info {
[12:23] <niemeyer>         var info *Info
[12:23] <niemeyer>         if c.subcmd != nil {
[12:23] <fwereade> niemeyer, as I recall we discussed this in some detail and it seemed to be the right way to deal with the legacy interspersed-or-not flag ickiness
[12:23] <niemeyer> fwereade: My reaction to it, IIRC, is that it was a bit on the clever side and practical, even if it was hard to understand
[12:24] <niemeyer> fwereade: Things are now being extended into other directions that extend the cleverness through the interface of Command, Parse, documenting recursiveness in the interface itself, etc
[12:24] <niemeyer> fwereade: I'm getting lost
[12:25] <niemeyer> fwereade: I totally admit it's probably my small brain, but I'd love if we could make this straightforward somehow
[12:25] <niemeyer> fwereade: It sounds simple just from the description of what we need
[12:25] <fwereade> niemeyer: the heart of it is in the requirement that we handle both "juju -v bootstrap" and "juju bootstrap -v"
[12:26] <rogpeppe> [13:24] <niemeyer> fwereade: Things are now being extended into other directions that extend the cleverness through the interface of Command, Parse, documenting recursiveness in the interface itself, etc
[12:26] <rogpeppe> +1
[12:26] <niemeyer> fwereade: Ok, but that *sounds* (and I may be missing details for sure) simple..
[12:27] <niemeyer> fwereade: The supercommand is wrapping the subcommands.. it is in control of what happens
[12:27] <niemeyer> fwereade: It knows there are only two possible layers, and it has the second layer entirely at its will
[12:28] <fwereade> niemeyer: to give you a bit of context on my thinking, would you read the comments on https://codereview.appspot.com/6107048/ and https://codereview.appspot.com/6100050/ please?
[12:30] <niemeyer> fwereade: I had read the first one before we started to talk.. checking the second
[12:33] <niemeyer> fwereade: Done
[12:33] <fwereade> niemeyer, is anything I'm saying sounding any saner?
[12:34] <niemeyer> fwereade: Everything you say is sane.. I just believe the problem may be solved in a simpler way
[12:35] <niemeyer> fwereade: I'll give it a quick shot
[12:35] <fwereade> niemeyer, before you do
[12:35] <niemeyer> fwereade: A hack, arguably, but hopefully it'll be helpful
[12:35] <fwereade> niemeyer, you seemed to like rog's proposal
[12:36] <niemeyer> fwereade: I don't think it's necessary either
[12:36] <fwereade> niemeyer, ok then :)
[12:37] <niemeyer> fwereade: Maybe it will be, though.. once we want to use supercommand with other globals
[12:37] <niemeyer> fwereade: But this is an aside.. the key point I'm making around non-recursiveness and non-looping is independent from it
[12:37] <fwereade> niemeyer, I feel that's a touch speculative
[12:38] <niemeyer> fwereade: Sure, I've been speculating since we first talked about this, and I'm now trying to stop speculating to show some code :)
[13:10] <rogpeppe> fwereade, niemeyer: here's a version of supercommand with no recursive magic: http://paste.ubuntu.com/944043/
[13:11] <TheMue> niemeyer: So, I now branched my last branch at the point before I modified the NeedsUpgrade behavior, but also before doing the changes of the according review. Do you expect a propose now to first take a look that this branch is in a proper state or shall I first do the review changes and then propose again?
[13:11] <rogpeppe> (branched from a slightly earlier version, so using ParsePositional, as i believe Consume was added only for the recursive thing)
[13:11] <fwereade> rogpeppe, how do you switch off logging?
[13:12] <niemeyer> TheMue: Feel free to do the review changes
[13:12] <rogpeppe> fwereade: the logging flags thing is orthogonal to this.
[13:12] <TheMue> niemeyer: ok
[13:12] <rogpeppe> fwereade: i still think my GlobalFlags proposal would work well
[13:12] <niemeyer> rogpeppe: Cheers.. I'm looking at this as well, as I mentioned
[13:13] <fwereade> rogpeppe, and, yes; you eliminate recursion by having a function that parses flags call a different function that parses flags in exactly the same way
[13:13] <rogpeppe> niemeyer: sorry, i'd already been playing with it, so thought i'd push out something to think about.
[13:13] <rogpeppe> fwereade: not exactly - it adds extra flags
[13:14] <niemeyer> rogpeppe: No worries.. I'm just trying to focus on it for a moment to understand the problem better
[13:14] <rogpeppe> fwereade: that's *the* crucial reason why SuperCommand exists, AFAICS
[13:14] <fwereade> rogpeppe, that it gets in the exact same way that Parse does...
[13:15] <fwereade> rogpeppe, there is repetition inherent in this problem
[13:15] <fwereade> rogpeppe, we can loop or we can recurse or we can duplicate code and kid ourselves that it's simpler because we've managed to avoid a loop
[13:15] <rogpeppe> fwereade: the entire reason for the recursion was so that we could reuse the 6 (!) lines of Parse?
[13:17] <fwereade> rogpeppe, well, there are many reasons all of which interrelate in one way or another, but yes, that is one of them
[13:17] <rogpeppe> fwereade: when nothing actually calls Parse itself other than Main, AFAICS. we could inline it into Main and noone would notice, i think.
[13:18] <rogpeppe> fwereade: i think the version of SuperCommand.ParsePositional i posted shows very directly the logic of SuperCommand.
[13:20] <fwereade> rogpeppe, yes it does, it looks to me very very much like the original version, except it wantonly duplicates the logic in Parse rather than just calling it; and it doesn't allow global args to come after the command, which was a requirement
[13:20] <rogpeppe> fwereade: it *does* allow global args to come after the command
[13:20] <rogpeppe> fwereade: it passes all tests
[13:22] <fwereade> rogpeppe, I don't see where you register the global flags for the second flagset Parse
[13:22] <rogpeppe> fwereade: newFlagSet
[13:23] <rogpeppe> fwereade: actually i was confused by that at first.
[13:23] <rogpeppe> fwereade: i'm not sure newFlagSet should take a Command, but that's another issue
[13:24] <fwereade> rogpeppe, well, the way you're using it makes it wrong; the way it was used before was fine ;)
[13:25] <rogpeppe> fwereade: ok, perhaps
[13:26] <fwereade> rogpeppe, still, I just do not get how repetition is not part of the problem
[13:26] <rogpeppe> fwereade: repetition of what?
[13:26] <fwereade> rogpeppe, and why I am being criticised for using elementary constructs which express repetition
[13:26] <fwereade> rogpeppe, repetition of parsing
[13:26] <fwereade> rogpeppe, one Command parses part of the command line and delegates to another for the rest
[13:27] <fwereade> rogpeppe, this is two parses; one would also be perfectly legitimate, because Parse should accept a Command
[13:28] <rogpeppe> fwereade: the repetition is performed once only, but you're making a very general mechanism to replace something that's uses exactly the same lines of code
[13:28] <niemeyer> fwereade: You're not being criticised.. I'm trying to simplify the logic we (the team) has in place.. that's impersonal
[13:28] <rogpeppe> niemeyer: +1
[13:29] <fwereade> niemeyer, rogpeppe: sorry, inappropriate language, I sometimes come to identify with my code a touch too much, ty for reminder
[13:29] <niemeyer> fwereade: Your code was largely reviewed and agreed with
[13:31] <rogpeppe> fwereade: this is all the diff it takes to make your code non-recursive and everything else can carry on using exactly the same interface before: http://paste.ubuntu.com/944063/
[13:31] <rogpeppe> fwereade: i don't see how that's not a win
[13:33] <fwereade> rogpeppe, ok, the internal implementation should not matter; do we agree that the real pain point is in returning things other than errors from ParsePositional
[13:33] <fwereade> ?
[13:33] <rogpeppe> fwereade: what do we want to return from ParsePositional?
[13:33] <fwereade> rogpeppe, well, an error, right? that's the interface you've been fighting to preserve
[13:34] <rogpeppe> fwereade: "the real pain point is in returning things other than errors"
[13:34] <rogpeppe> fwereade: so what else do we want to return?
[13:35] <fwereade> rogpeppe: I'm starting to think: a command and list of unconsumed args (which replaces ourself entirely if used and is in practice nil, nil for most Commands, although there are other plausible uses of such a mechanism)
[13:35] <fwereade> rogpeppe, plus an error
[13:35] <rogpeppe> fwereade: but why?
[13:36] <fwereade> rogpeppe, because it is a perfect fit for the problem space and reduces duplication
[13:36] <rogpeppe> fwereade: we're talking 6 lines of duplication here!
[13:36] <rogpeppe> fwereade: and you're proposing to change the interface in many places just because of that?
[13:36] <fwereade> rogpeppe, and you're saying that 6 lines of duplication is a *better* thing than a loop to express the same repetition?
[13:37] <rogpeppe> fwereade: yes
[13:38] <hazmat> g'morning
[13:38]  * hazmat is glad to finally be home
[13:38] <fwereade> rogpeppe, in and of itself that makes little sense to me; with the wider context of "because it allows us to have a cleaner signature for ParsePositional" I can understand it
[13:38] <fwereade> heya hazmat
[13:38] <rogpeppe> fwereade: {fmt.Println("a"); fmt.Println("b")} can be better than for _, s := range []string{"a", b"} {fmt.Println(s)}
[13:38] <rogpeppe> hazmat: hiya
[13:39] <rogpeppe> fwereade: and in this case, it's not strict repetition - the body of the loop is slightly different each time through.
[13:39] <niemeyer> fwereade: FWIW, I'm still on it..
[13:43] <rogpeppe> fwereade: if it *was* a simple loop, i wouldn't mind much. my problem is with the fact that to avoid this repetition you're proposing complicating the Command interface that's implemented everywhere.
[13:48] <fwereade> rogpeppe, where by "everywhere" you mean "in two places"?
[13:49] <fwereade> rogpeppe, hm, sorry, suddenly unsure whether you're looking at the version with FlagCommand
[13:49] <fwereade> rogpeppe, ok, 3 places, FlagCommand implements it ;)
[13:49] <rogpeppe> fwereade: ah, i'm not.
[13:49] <rogpeppe> fwereade: but still.
[13:50] <rogpeppe> fwereade: it's implemented (even if only with embedding) by every subcommand
[13:50] <fwereade> rogpeppe, I guess this may be one of the sources of disconnect; my position is that the prectical cost is utterly minimal
[13:51] <rogpeppe> fwereade: the Command interface is something that people will see when maintaining the code. it's a very good thing that it's as simple as possible.
[13:52] <rogpeppe> fwereade: both myself and niemeyer found the recursive nature of supercommand difficult to understand.
[13:52] <fwereade> rogpeppe, I thought one of the benefits of Reduce was that it actually made everything cleaner
[13:52] <rogpeppe> fwereade: sometimes 6 lines of extra code are worth one less level of abstraction IMHO
[13:53] <rogpeppe> fwereade: i think everything's cleaner if *all* the supercommand magic is strictly within supercommand
[13:54] <rogpeppe> fwereade: in fact, the patch above isolates all the magic into SuperCommand.ParsePositional - a few lines of sequential code.
[13:56] <rogpeppe> fwereade: FWIW i don't think i've seen a CL with FlagCommand in it
[13:57] <fwereade> rogpeppe, it's in https://codereview.appspot.com/6107048/
[13:58] <fwereade> rogpeppe, niemeyer proposes something better but I haven't reproposed with that in
[14:00] <rogpeppe> fwereade: even with ZeroArgsConsumer, i still see 5 implementations of Consume
[14:02] <fwereade> rogpeppe, you mean including places where ZeroArgsConsumer is embedded?
[14:02] <rogpeppe> fwereade: no
[14:02] <rogpeppe> e.g. func (c *agentConf) Consume(args []string) ([]string, error) {
[14:03] <rogpeppe> fwereade: every time someone sees that method, they have to wonder: does it actually return something other than an empty arg list?
[14:03] <niemeyer> Strawman on the way..
[14:04] <rogpeppe> fwereade: and the only reason for it is SuperCommand
[14:04]  * rogpeppe likes the nutty, chewy taste of straw
[14:05] <fwereade> rogpeppe, ah, typo, I was mid-edit :/ ...sorry :)
[14:05] <niemeyer> https://codereview.appspot.com/6118044
[14:06] <niemeyer> Tests are broken, but it actually builds at least
[14:09] <rogpeppe> niemeyer: i like the look of that
[14:09] <rogpeppe> niemeyer: particularly the way lines of code melt away :-)
[14:09] <rogpeppe> niemeyer: not sure about calling FlagSet.Parse twice on the same flag set, but i guess it might be ok.
[14:10] <niemeyer> rogpeppe: We can always make it ok :)
[14:10] <niemeyer> rogpeppe: But I suspect it's already fine
[14:10] <rogpeppe> niemeyer: yeah, or just initialise a new flagset
[14:11] <niemeyer> rogpeppe: I'd rather not, if possible.. I really like how we have a single place where this is happening
[14:11] <rogpeppe> niemeyer: that's true. it removes a lot of head-scratching.
[14:17] <fwereade> niemeyer, that's *very* nice indeed
[14:18] <fwereade> niemeyer, almost horrifyingly so
[14:24] <niemeyer> fwereade: Cheers
[14:25] <fwereade> niemeyer, I'm still trying to think through how it'll work with the responsibility split but I think it goes in a nice direction there too
[14:26] <niemeyer> fwereade: Yeah, I suspect it should be fine
[14:26] <fwereade> niemeyer, ok, I'll take a short break and get onto it
[14:26] <rogpeppe> niemeyer, fwereade, TheMue: are we still planning to have a meeting this afternoon?
[14:26] <niemeyer> fwereade: ctx takes care of most of the issue there already, I believe
[14:26] <fwereade> oh, yeah, good plan
[14:26] <niemeyer> rogpeppe: +1
[14:26] <niemeyer> I'm happy to have it *now*
[14:27] <rogpeppe> i've got a 1 to 1 with robbie in 5 minutes, but i'm good after that
[14:27] <niemeyer> I have only about half an hour, but should be useful at least
[14:27] <TheMue> rogpeppe: In half an hour I've got my 1:1 with Robbie.
[14:27] <niemeyer> rogpeppe: Aw, ok
[14:27] <niemeyer> rogpeppe: After lunch then
[14:27] <niemeyer> in 1h30?
[14:27] <rogpeppe> niemeyer: i'm sure i could defer with robbbiew
[14:27] <niemeyer> rogpeppe: I'm short..
[14:27] <niemeyer> rogpeppe: 1h30 would work best there too
[14:27] <rogpeppe> niemeyer: sounds good to me
[14:28] <niemeyer> 16UTC then
[14:28] <rogpeppe> niemeyer: yup.
[14:28] <TheMue> niemeyer: OK
[14:28] <niemeyer> fwereade, TheMue: Sounds good?
[14:29] <fwereade> niemeyer, 1h30 from now sounds good
[14:30] <niemeyer> Super
[14:32] <robbiew> rogpeppe: we 1:1ing? or did you defer me :/
[14:32] <robbiew> :P
[14:33] <rogpeppe> robbiew: we are
[14:33] <rogpeppe> robbiew: G+?
[14:34] <robbiew> rogpeppe: aye
[14:34] <robbiew> https://talkgadget.google.com/hangouts/_/extras/canonical.com/the-hulk-hideout
[14:34] <robbiew> rogpeppe: ^
[14:37] <TheMue> niemeyer: Please forget latest propose. Somehow the merge of parent changes brought unwanted stuff in. Seems I took the wrong revision.
[14:46] <niemeyer> TheMue: np
[14:49] <TheMue> niemeyer: I've got to admit that I'm a bit puzzled while trying to understand which revision I should take. I opened it in Bazaar Explorer to get a better impression. But I didn't thought that the needed merge to propose the stuff would get my not-wanted code back. *sigh*
[15:13]  * niemeyer => lunch
[15:14] <TheMue> niemeyer: So, now I've rolled back to an earlier revision, made the needed changes and would now like to propose it. How can this be done without the problem of a "diverged-branch"?
[15:14] <TheMue> niemeyer: Oh, lunch, enjoy. ;)
[15:21] <rogpeppe> TheMue: you need to have merged with trunk
[15:22] <rogpeppe> TheMue: if bzr diff looks right, then the codereview should look right, assuming you've got the prereq correct
[15:23] <rogpeppe> TheMue: also, i didn't know this for a while: you can't change the prereq of an existing merge proposal - you have to delete and repropose if you want to do that
[15:23] <TheMue> rogpeppe: aha, have to take a look
[15:25] <TheMue> rogpeppe: How do I do it?
[15:26] <rogpeppe> TheMue: there's a "Delete this proposal" link in the top right of the merge proposal page
[15:28] <TheMue> rogpeppe: You mean "Delete patch set"?
[15:28] <TheMue> rogpeppe: Otherwise I'm on a different page. *lol*
[15:29] <rogpeppe> TheMue: this page: https://code.launchpad.net/~themue/juju/go-state-unit-resolved-watcher/+merge/102497
[15:29] <rogpeppe> TheMue: "Delete proposal to merge"
[15:30] <TheMue> rogpeppe: Ah, thx, I've been on the review page.
[15:31] <rogpeppe> TheMue: the launchpad pages are the important thing.
[15:31] <rogpeppe> TheMue: you don't want to delete the codereview page.
[15:32] <TheMue> rogpeppe: I just hoped both play together.
[15:33] <rogpeppe> TheMue: codereview.com knows nothing about launchpad.net or vice versa. the only link is lbox.
[15:34] <TheMue> rogpeppe: So we need a "lbox sync" supercommand. ;)
[15:35] <rogpeppe> TheMue: there's no change you can make in codereview that we'd want to sync to launchpad, other than comments, which get transferred anyway.
[15:35] <rogpeppe> TheMue: so lbox is already like lbox sync, just in one direction
[15:36] <TheMue> rogpeppe: OK, to make it more clear. <JOKE> So we need a … </JOKE>.
[15:36] <rogpeppe> TheMue: oh
[15:36] <rogpeppe> TheMue: ha ha :-)
[15:36] <TheMue> rogpeppe: I've got to admit it has been a bad one. ;)
[15:37] <TheMue> rogpeppe: Just hoped to get a link into the supercommand discusion today.
[15:37] <rogpeppe> TheMue: what kind of link?
[15:38] <TheMue> rogpeppe: Hmm, maybe wrong wording. How would say it if you want to connect a sentence now to a discussion that has been helt some time ago?
[15:38] <TheMue> rogpeppe: A connection?
[15:39] <rogpeppe> TheMue: you'd probably say something like "just hoped to make a reference to the supercommand discussion today"
[15:39] <rogpeppe> "reference" is the word
[15:40] <TheMue> rogpeppe: Ah, ok, thx, learned again. Sounds a bit formal from a German perspective.
[15:41] <TheMue> TheMue: Here we also have the word "Referenz", but it is used in a different context.
[15:54] <rogpeppe> TheMue: alternative phrasing "i was referring to the supercommand discussion" or "i was trying to refer to ..."
[15:57] <TheMue> rogpeppe: Sounds like the same stem. How would you say it if you're in the context of making a joke? Would you also "refer to"?
[15:58]  * TheMue likes English lessons here in the channel, as long as I don't have to pay for.
[15:59] <rogpeppe> TheMue: probably not in a joke itself. i'd just use some phrasing that alludes to the original thing.
[16:00] <TheMue> rogpeppe: I tried it with the "lbox sync" supercommand, but yes, that hasn't been funny.
[16:00] <rogpeppe> TheMue: i can't say i laughed out loud :-)
[16:01] <TheMue> rogpeppe: I'll try to make a better one next time.
[16:01] <rogpeppe> TheMue: i wait with eager anticipation
[16:02] <TheMue> rogpeppe: I hope I won't dissappoint you.
[16:03]  * niemeyer is back
[16:04] <rogpeppe> niemeyer, fwereade, TheMue: meeting?
[16:04] <TheMue> rogpeppe: I still have the "diverged-branches" problem.
[16:04] <rogpeppe> TheMue: push --overwrite
[16:04] <niemeyer> yep!
[16:04] <niemeyer> rogpeppe: Wanna do the honors?
[16:06] <rogpeppe> niemeyer: k
[16:07] <rogpeppe> niemeyer, TheMue, fwereade: invites out
[16:11]  * hazmat lunches
[16:18] <niemeyer> Lost my connection.. trying again
[16:19] <niemeyer> rogpeppe: WTF.. is everybody else in?
[16:19] <rogpeppe> niemeyer: yeah
[16:19] <rogpeppe> niemeyer: we're still there
[16:19] <niemeyer> rogpeppe: I'm retrying but it shows me as being online in my own list..
[16:20] <rogpeppe> niemeyer: you're there, but in cartoon form only
[16:20] <niemeyer> rogpeppe: I can hear you typing
[16:20] <rogpeppe> niemeyer: lol
[16:20] <niemeyer> Come on G+!
[16:20] <niemeyer> I can hear everybody
[16:20] <niemeyer> I'm disconnecting and retrying
[16:21] <rogpeppe> can you use the text chat box?
[16:21] <niemeyer> rogpeppe: I could..  just disconnected.. will retry
[16:21] <niemeyer> Killing Chrome and restarting
[16:25] <niemeyer> Shait
[16:25] <rogpeppe> niemeyer: dammit
[16:26] <niemeyer> This software wasn't so crackful before
[16:27] <rogpeppe> niemeyer: it's working fine for us :-)
[16:27] <niemeyer> rogpeppe: That's the kind of trick that always works.. if you restrict what "us" means enough, it everything is wonderful! ;-)
[16:28] <rogpeppe> niemeyer: us == ∅ :-)
[16:30] <rogpeppe> niemeyer: maybe if you invited us, the server might be located in a better place...
[16:30] <niemeyer> Ok, let's try that
[16:32] <niemeyer> G+ got frozen completely and Chrome had to kill the page
[16:32] <niemeyer> It's that bad..
[16:33] <niemeyer> "The connection to talkgadget.google.com was interrupted."
[16:33] <rogpeppe> niemeyer: hmm. i wonder if it's a networking issue at all. maybe you could try connecting using a Windows machine :-) :-)
[16:33] <TheMue> rogpeppe: Evil
[16:34] <niemeyer> rogpeppe: Oh yeah, I'm sure it's the kernel!
[16:35] <niemeyer> robbiew: ping
[16:35] <robbiew> pong
[16:35] <fwereade> niemeyer, I'm not seeing a new invite; should I be?
[16:36] <niemeyer> robbiew: Can we borrow your conf call number for a moment?
[16:36] <robbiew> one sec
[16:36] <niemeyer> I can't find my pin
[16:37] <niemeyer> fwereade: No.. it's seriously broken
[16:37] <niemeyer> fwereade: I get this on the tab in the G+ page: The connection to talkgadget.google.com was interrupted.
[16:37] <fwereade> niemeyer, ouch, didn't realise that was the main G+ page
[16:38] <niemeyer> fwereade: <robbiew> niemeyer: 107 568 4916
[16:38] <niemeyer> This is the conf code..
[16:38] <niemeyer> Phone number is at /ConferenceCalls on the internal wiki
[16:47] <rogpeppe> fwereade, TheMue: you might want to add an additional communication channel...
[16:54] <fwereade> rogpeppe, sorry, what should I drop out of?
[17:15] <fwereade> niemeyer, rogpeppe, TheMue: I'm afraid I should be away soon in the interest of domestic harmony...
[17:17] <fwereade> niemeyer, rogpeppe, TheMue: and I'm afraid I certainly can't do justice to the details of this conversation
[17:17] <fwereade> happy evenings all
[17:17] <TheMue> fwereade: bye
[17:19] <niemeyer> fwereade: Sounds good, we certainly don't want to disturb the domestic harmony there ;-)
[17:26] <niemeyer> TheMue: I was just looking at GoPort.. I'm not sure about how useful that is ATM.. maybe you have different plans for it, but right now it feels like a snapshot of the +activereviews link in Launchpad
[17:26] <niemeyer> TheMue: What I had in mind was closer to a "bird's view" on the different areas
[17:26] <niemeyer> TheMue: Something vague enough to be up-to-date for longer
[17:27] <niemeyer> TheMue: Also, re. https://code.launchpad.net/~themue/juju/go/+merge/103315, do you want me to review that or was it just a test?
[17:28] <niemeyer> TheMue: Wondering mostly due to the branch name
[17:51] <andrewsmedina> niemeyer, rogpeppe please take a look https://codereview.appspot.com/6099051/ :-p
[17:54] <niemeyer> andrewsmedina: Woohay
[18:11] <TheMue> niemeyer: The GoPort page is currently only a start. It shall also contain all open features. It's like a Kanban board, only in a wki.
[18:12] <TheMue> niemeyer: Tools based it would be simpler.
[18:13] <TheMue> niemeyer: The last branch is for review. The name has gone so "bad" due to an error while fighting with bzr and lbox.
[18:14] <niemeyer> TheMue: Just saying.. I'm not going to be joining you on that style of roadmap
[18:14] <niemeyer> TheMue: This is what +activereviews provides
[18:15] <niemeyer> TheMue: and changes every day
[18:15] <niemeyer> TheMue: It won't help
[18:15] <TheMue> niemeyer: Where in activereviews are all outstanding features?
[18:15] <niemeyer> TheMue: Where in GoPort are all outstanding features?
[18:15] <niemeyer> TheMue: What's there now is a list of branches
[18:15] <niemeyer> TheMue: that's what I'm talking about
[18:16] <TheMue> niemeyer: I already told you that I'm just started to get a feeling how to put it into the wiki. When the form is right I would talk to you and all others to gather the outstanding the features.
[18:17] <niemeyer> TheMue: Sure.. you mentioned that page in the wiki today, I've looked, and am providing feedback on what's there.. no biggie
[18:17] <TheMue> niemeyer: I'm not yet habe with the feature naming too. And I've got to try what's the best granularity.
[18:17] <niemeyer> TheMue: As I said, I'm also happy to try to produce something you're happy with
[18:17] <TheMue> niemeyer: So any good idea by you is welcome.
[18:18] <niemeyer> TheMue: I'd have to sit down and do it.. which is fine by me, but it won't look like what's there now
[18:19] <niemeyer> TheMue: I'd probably take the big areas from the Python code, and say what's missing at a very high level
[18:19] <TheMue> niemeyer: I'm also not absolutely happy. It's not simple to put it into a wik iform. In past we used a mix of tools and physical kanban boards.
[18:20] <TheMue> niemeyer: Yes, I already started scanning it. Today it's indeed to fine granular and the orientation at branches isn't right.
[18:20] <niemeyer> TheMue: I've used a number of different techniques too.. we have tools that produce something resembling a Kanban from the reviews actually
[18:21] <TheMue> niemeyer: Nice
[18:21] <TheMue> niemeyer: And you feed it by creating LP issues for open points?
[18:21] <niemeyer> TheMue: E.g. http://people.canonical.com/~niemeyer/florence.html
[18:22] <TheMue> niemeyer: Great, want. *smile*
[18:22] <niemeyer> TheMue: We also had feature boards in the wiki with Landscape
[18:22] <niemeyer> TheMue: But I don't miss either of these right now
[18:22] <niemeyer> TheMue: I'm happy to look at the problems we're trying to solve and come up with another solution that fits for what we're doing today
[18:23] <niemeyer> TheMue: UDS will be a good time for the two of us to sit together and come up with a plan
[18:24] <TheMue> niemeyer: Yes, looking forward. Maybe we really don't need this way of control, only a list of milestones ensuring that we don't miss the finishing at 12.10.
[18:24] <niemeyer> TheMue: Yeah.. and some big areas that we can keep in mind and know how much progress we have there
[18:25] <niemeyer> TheMue: E.g. "provisioning agent", "unit agent", "machine agent"
[18:25] <TheMue> niemeyer: Sounds good. I really don't want to create big, ugly docs just for fun.
[18:25] <niemeyer> TheMue: "state", "ec2 provider", ...
[18:26] <TheMue> niemeyer: As you have seen I've also put some first open questions/topcs at the bottom. At least open for me.
[18:26] <niemeyer> "local provider", /me looks at andrewsmedina
[18:26] <TheMue> niemeyer: Maybe some of them are only interesting for > 12.10, but some maybe earlier
[18:26] <niemeyer> "command line API"
[18:26] <niemeyer> TheMue: I'd not mix that up
[18:27] <niemeyer> TheMue: Or we'll risk losing focus
[18:27] <TheMue> niemeyer: Oh, good that this stuff is logged. I'll add it tomorrow. Sounds even better than todays table.
[18:27] <niemeyer> TheMue: We need sharp attention on the port.. everything else is secondary
[18:28] <niemeyer> TheMue: I'd put each of these as headers of a section
[18:28] <TheMue> niemeyer: Eh, I only wrotem them down to make clear that those parts which are important for 12.10 are handled early enough. Others may be deferred.
[18:29] <TheMue> niemeyer: If you signal "This point A is covered there, and that point B is not interesting for the 12.10 Go port." than it's ok.
[18:29] <TheMue> niemeyer: Nothing more.
[18:39] <andrewsmedina> niemeyer: what?
[18:53] <TheMue> Off for today.
[18:53]  * TheMue waves
[19:08] <niemeyer> andrewsmedina: nm
[19:08]  * niemeyer breaks for a bit
[19:11] <andrewsmedina> niemeyer: you saw my review?
[19:35] <niemeyer> andrewsmedina: Not yet, but we'll get to it for usre
[19:35] <niemeyer> sure
[19:49] <hazmat> niemeyer, are you going to hit mongosv next week in sf?
[21:33] <hazmat> bcsaller, i'm realizing that juju-info should probably also include some additional info relating to charm name and interfaces
[21:33] <hazmat> take a monitoring tool, how's it know what its monitoring outside of specific monitoring support in the monitored charm
[21:34] <bcsaller> hazmat: I would think anything it includes could be included in any/all charms
[21:34] <bcsaller> hazmat: and by charms I mean relations
[21:38] <hazmat> hmm.. i suppose so, but this case implicit where as the others are already explicit, but given a polymorphic interface i suppose that makes sense as well
[21:39] <bcsaller> hazmat: more info and fewer special cases, at minimum there is one win in that list
[23:23] <SpamapS> hazmat: +1 from me to adding charm and service name to all relations
[23:23] <SpamapS> easy win, lots less special casing
[23:28] <hazmat> SpamapS, true, but adding it to the all rels feels like we're missing something its going to duplicated for every remote unit.
[23:29] <hazmat> its really a remote endpoint property
[23:29] <hazmat> but we don't have a nice way to expose that except on via relation-get on the the remote unit
[23:31] <SpamapS> hazmat: hm, not sure I understand. You're concerned that its the same for all units and so it is a different type of information.. but I think thats ok because its always true for each unit even though it is redundant.
[23:36] <hazmat> SpamapS, yeah.. just wanted to see if we could store it differently to avoid the copies
[23:42] <SpamapS> hazmat: why would it be stored?
[23:42] <SpamapS> hazmat: we should have lazy-fetch for stuff like that
[23:45] <hazmat> SpamapS, rel-get doesn't differentiate on keys, the info could be store on the rel, as for why store on the rel, i've been trying to keep contexts fully functional even without topo info to minimize issues arising from lack of stop hooks
[23:45] <hazmat> ie. self contained for rel hook exec
[23:45] <hazmat> but that's a losing proposition anyways