babbageclunk | thumper: fix for the watcher tests: https://github.com/juju/juju/pull/10573 | 00:46 |
---|---|---|
babbageclunk | I don't really understand why it works though - shouldn't the call to StartSync in the NotifyWatcherC have the same effect? | 00:46 |
thumper | babbageclunk: do you want me to explain why? | 01:55 |
thumper | babbageclunk: and there is a better method | 01:56 |
babbageclunk | thumper: yes please! | 02:02 |
thumper | babbageclunk: otp now, see PR | 02:03 |
babbageclunk | ok, that's clear - thanks! | 02:03 |
babbageclunk | updating now | 02:03 |
wallyworld | anastasiamac: any chance to look at those PRs? i've tweaked the use of ctx.out in the first one | 02:40 |
* anastasiamac looking | 02:41 | |
anastasiamac | wallyworld: almost :) i think it's painfully close... commented | 02:53 |
wallyworld | anastasiamac: ty, will look real soon after meeting | 02:54 |
anastasiamac | nws. m not in rush \o/ | 02:54 |
thumper | babbageclunk: https://github.com/juju/juju/pull/10573 wasn't approved, so your merge didn't happen | 03:00 |
thumper | I have approved it now | 03:00 |
=== exsdev0 is now known as exsdev | ||
wallyworld | anastasiamac: using a struct for args results in different output to the spec (the Arguments header has a ":" instead of free text). And there's still the need to print the action description as free text | 03:45 |
wallyworld | maybe the extra ":" doesn't matter | 03:46 |
anastasiamac | wallyworld: yeah i would not worry about ':" | 03:47 |
anastasiamac | :D | 03:47 |
wallyworld | i also like the extra \n at the end | 03:48 |
wallyworld | otherwise it's hard to see where the output ends and the prompt starts | 03:48 |
anastasiamac | but inconsistent with all other commands, especially show- ones | 03:49 |
wallyworld | the other ones are wrong then :-) | 03:49 |
anastasiamac | haha | 03:52 |
wallyworld | anastasiamac: there's still the matter of some free text being needed for the description. that still requires fprintf | 03:53 |
wallyworld | in which case may as well use fprintf for the "Arguments" header as well | 03:54 |
wallyworld | unless we insist on the use of "Description:" | 03:56 |
anastasiamac | wallyworld: m wondering if u r really thinking this or just not keen to change the PR...:) fwiw m happy with it to land as is but i think it has these rough edges that could have been avoided from the start.... | 03:56 |
anastasiamac | wallyworld: i do not think that "DEscription:" is bad :) | 03:56 |
wallyworld | what rough edges? just sticking to the spec | 03:56 |
wallyworld | the spec calls for description as free text | 03:56 |
wallyworld | i can see why that is not a bad thing as it's user readable | 03:57 |
anastasiamac | like too much fmt.Printf directly to stderr and stdout - one of the things i want to tackle next and it'd be (and laready is a nightmare) coz of the rogue direct writes to stderr/stdout | 03:57 |
wallyworld | it just so happens the printing of the action args is "yaml-like" | 03:57 |
anastasiamac | already* | 03:57 |
anastasiamac | want to talk it over rather than type? | 03:58 |
wallyworld | sure | 03:58 |
anastasiamac | stdup? | 03:59 |
wallyworld | ok | 03:59 |
babbageclunk | thumper: ah, right - thanks! | 04:23 |
wallyworld | anastasiamac: https://github.com/juju/juju/pull/10568 should be ok now, has the rename | 04:42 |
anastasiamac | yes i saw the update and m looking already :D | 04:43 |
wallyworld | you are on the ball | 04:43 |
anastasiamac | haha yes u do keep me on my toes :) | 04:48 |
=== hml_ is now known as hml | ||
rick_h | hml: looks like the build came through this time so woot woot | 13:46 |
hml | rick_h: rgr | 13:46 |
achilleasa | hml or stickupkid: can either of you please take a look at https://github.com/juju/packaging/pull/7? | 14:02 |
stickupkid | achilleasa, looking | 14:07 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!