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