[08:07] <Chipaca> ｇｏｏｏｏｏｏｏｏｄ  ｍｏｒｎｉｎｇ！
[08:08] <bthomas> 𝐌𝐨𝐫𝐧𝐢𝐧𝐠
[08:08] <bthomas>  
[08:08] <bthomas>  
[08:08] <bthomas>  
[08:08] <bthomas> hmmmm... unifonter added an extra newline
[08:10] <Chipaca> i need to reboot for a bios upgrade apparently
[08:10] <Chipaca> s/bios/firmware/
[08:10] <Chipaca> bbiab
[08:16] <Chipaca> 𝑤𝑓𝑚
[08:17] <Chipaca> bthomas: ^ no spurious newline
[08:17] <bthomas> Chipaca: I do C-u M-! unifonter -k ...
[08:18] <bthomas> 𝑡𝑟𝑦𝑖𝑛𝑔 𝑎𝑔𝑎𝑖𝑛
[08:18] <bthomas>  
[08:20] <Chipaca> bthomas: are you running unifonter from the snap?
[08:20] <bthomas> Ah no. Why should that make a difference ?
[08:20] <bthomas> I might also be on an old version.
[08:20] <Chipaca> bthomas: if you've tweaked the binary, i have no idea what it does :)
[08:20] <bthomas> Chipaca: fair enough
[08:20] <Chipaca> bthomas: C-u M-! unifonter  does not add extra newlines here
[08:28] <Chipaca> bthomas: not saying it's not possible it's doing something wrong, but would like to be sure we're talking about the same code before i start hunting :)
[08:29] <bthomas> Chipaca: will look into it this evening. Most likely I messed about unifonter code when I was trying to read and play with it. What is the point of open source if you do not do that :-)
[08:29] <Chipaca> bthomas: oh totally :)
[09:54] <Chipaca> jam: morning!
[10:36]  * Chipaca steps out for a bit
[10:46] <jam> morning Chipaca
[10:57] <facubatista> ¡Muy buenos días a todos!
[11:01] <bthomas> नमस्ते facubatista
[11:01] <facubatista> hola bthomas
[11:01] <bthomas> Morning jam
[11:02] <bthomas> Just finished writing all unit tests I can think of for prometheus charm. Only docs strings and comments are left. Will switch to writing operator hook docs post lunch.
[11:12] <facubatista> bthomas, Chipaca, can I get reviews here, please? thanks https://github.com/canonical/charmcraft/pull/146
[11:12] <mup> PR charmcraft#146: Removed support for names transition in some Store responses <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/146>
[11:23] <bthomas> facubatista: Had a look at pull 146. LGTM how ever I do not have knowledge of this codebase or problem domain. So my approval is only worth that much.
[11:23] <mup> PR #146: Testing harness <Created by jameinel> <Merged by jameinel> <https://github.com/canonical/operator/pull/146>
[11:28] <facubatista> bthomas, thanks
[13:50] <Chipaca> facubatista: charmcraft#146 gtg
[13:50] <mup> PR charmcraft#146: Removed support for names transition in some Store responses <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/146>
[13:54] <Chipaca> facubatista: I'll merge it now, ok?
[13:56]  * Chipaca takes that as a yes
[13:57] <facubatista> Chipaca, yes, thanks
[14:14] <Chipaca> facubatista: charmcraft#155 is so much easier to review now :-D
[14:14] <mup> PR charmcraft#155: Produce the default help text <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/155>
[14:15] <Chipaca> facubatista: charmcraft#150 now has tests, please re-review
[14:15] <mup> PR charmcraft#150: when failing to create hard link attempt to use shutil.copy2 <Created by matuskosut> <https://github.com/canonical/charmcraft/pull/150>
[14:16] <facubatista> Chipaca, there's no "this PR depends on this other one" so it fixes the diffs, as we have in LP, right?
[14:16] <Chipaca> facubatista: correct
[14:17] <facubatista> silly github
[14:17] <Chipaca> facubatista: you can get diffs between any two branches though, so that helps
[14:17] <Chipaca> related branches*
[14:18] <Chipaca> you can't diff https://github.com/torvalds/linux and https://github.com/python/cpython
[14:22] <facubatista> makes sense :)
[14:23]  * facubatista wants now to have a https://github.com/batista/facundox
[14:23] <facubatista> Chipaca, need to improve tests in charmcraft#150
[14:23] <mup> PR charmcraft#150: when failing to create hard link attempt to use shutil.copy2 <Created by matuskosut> <https://github.com/canonical/charmcraft/pull/150>
[14:23] <Chipaca> yup, you're of course correct
[14:29] <facubatista> bthomas, can I get also a review on https://github.com/canonical/charmcraft/pull/155 ? thanks!
[14:29] <mup> PR charmcraft#155: Produce the default help text <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/155>
[14:39] <bthomas> done
[17:24] <facubatista> bthomas, Chipaca, the rest of the builders, thanks for the review! https://github.com/canonical/charmcraft/pull/156
[17:24] <mup> PR charmcraft#156: Text builders for simple usage, detailed help, and command help <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/156>
[17:26] <Chipaca> facubatista: ack
[17:28] <Chipaca> facubatista: one criticism re your review of charmcraft#150: you're holding a code contribution from an external developer to a higher standard than the current code :-/
[17:28] <mup> PR charmcraft#150: when failing to create hard link attempt to use shutil.copy2 <Created by matuskosut> <https://github.com/canonical/charmcraft/pull/150>
[17:29] <Chipaca> facubatista: i agree with your review, and i've fixed the issue (and i guess you could say the higher-standard thing only really comes in when talking about my changes to the 3rd-party code), but
[17:29] <Chipaca> eso
[17:30] <facubatista> Chipaca, but current code also checks permissions when relevant, right?
[17:31] <Chipaca> facubatista: the test_build_generics_tree at least just checks for existence, not that they're the same
[17:31] <Chipaca> test_build_generics_simple_files checks that they're the same inode, which i hadn't noticed
[17:32] <facubatista> right, if it's the same inode everything is the same
[17:32] <Chipaca> and test_build_generics_simple_dir checks the st_mode
[17:32] <facubatista> my point is that if we copy stuff and remove it's exec bit (for example) we may be breaking the charm
[17:33] <Chipaca> facubatista: as i say, i don't disagree that we need to check this stuff :)
[17:33] <Chipaca> in fact, re-review plz :)
[17:34] <Chipaca> facubatista: two files can have the same inode and be different though 🙂
[17:34]  * Chipaca is being suuuuper picky and silly tbh
[17:34] <Chipaca> facubatista: (because you check st_ino and not st_dev)
[17:35] <facubatista> Chipaca, oh, that's another reason for the `os.link` to fail, right?
[17:35] <facubatista> I mean, fallbacking to copying the file is good for that multiple-devices case
[17:36] <facubatista> that would be rare, though, people mounting other devices inside the charm structure
[17:37] <Chipaca> facubatista: that would be an OSError and not a PermissionError though, but yes
[17:37] <Chipaca> facubatista: made tricky by PermissionError being an OSError
[17:38] <Chipaca> it's an OSError with errno==18
[17:38] <Chipaca> the xlink one i mean
[17:39] <Chipaca> facubatista: i could see somebody trying to mount an outside volume into multipass' build/ for ex
[17:40] <Chipaca> facubatista: i'll add a test and a little bit of code to handle it just in case
[17:42] <facubatista> Chipaca, ack
[17:47] <Chipaca> facubatista: the st_atime check sometimes fails :-/
[17:48] <facubatista> Chipaca, "times" are important?
[17:49] <Chipaca> i can't think of a case of the atime to be important
[17:49] <Chipaca> (or the same but in english)
[17:50] <Chipaca> and of course now i can't reproduce the failure
[17:50] <facubatista> the trap of testing times :p
[17:51] <Chipaca> facubatista: pushed one more commit, hth
[17:51]  * Chipaca gets dinner in th'oven
[18:08]  * facubatista -> tennis
[20:52]  * facubatista is back
[21:57] <mup> PR operator#403 opened: Make Harness load default values from config.yaml <Created by johnsca> <https://github.com/canonical/operator/pull/403>
[22:52] <hloeung> facubatista: thanks for your review, updated MP with fixes!
[22:53] <facubatista> hloeung, thank you!
[22:56] <hloeung> facubatista: oh hai, when you have a moment, can you go over and re-review / approve? Thanks
[22:56] <facubatista> hloeung, I will, tomorrow
[22:57] <facubatista> I'm eoding right now (well, after your review, it was)
[22:57] <hloeung> yep yep, works for me