=== xavpaice- is now known as xavpaice
Chipacagooooooood  morning!08:07
bthomas 08:08
bthomas 08:08
bthomas 08:08
bthomashmmmm... unifonter added an extra newline08:08
Chipacai need to reboot for a bios upgrade apparently08:10
Chipacabthomas: ^ no spurious newline08:17
bthomasChipaca: I do C-u M-! unifonter -k ...08:17
bthomas𝑡𝑟𝑦𝑖𝑛𝑔 𝑎𝑔𝑎𝑖𝑛08:18
bthomas 08:18
Chipacabthomas: are you running unifonter from the snap?08:20
bthomasAh no. Why should that make a difference ?08:20
bthomasI might also be on an old version.08:20
Chipacabthomas: if you've tweaked the binary, i have no idea what it does :)08:20
bthomasChipaca: fair enough08:20
Chipacabthomas: C-u M-! unifonter  does not add extra newlines here08:20
Chipacabthomas: 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:28
bthomasChipaca: 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
Chipacabthomas: oh totally :)08:29
Chipacajam: morning!09:54
* Chipaca steps out for a bit10:36
jammorning Chipaca10:46
facubatista¡Muy buenos días a todos!10:57
bthomasनमस्ते facubatista11:01
facubatistahola bthomas11:01
bthomasMorning jam11:01
bthomasJust 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:02
facubatistabthomas, Chipaca, can I get reviews here, please? thanks https://github.com/canonical/charmcraft/pull/14611:12
mupPR charmcraft#146: Removed support for names transition in some Store responses <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/146>11:12
bthomasfacubatista: 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
mupPR #146: Testing harness <Created by jameinel> <Merged by jameinel> <https://github.com/canonical/operator/pull/146>11:23
facubatistabthomas, thanks11:28
Chipacafacubatista: charmcraft#146 gtg13:50
mupPR charmcraft#146: Removed support for names transition in some Store responses <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/146>13:50
Chipacafacubatista: I'll merge it now, ok?13:54
* Chipaca takes that as a yes13:56
facubatistaChipaca, yes, thanks13:57
Chipacafacubatista: charmcraft#155 is so much easier to review now :-D14:14
mupPR charmcraft#155: Produce the default help text <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/155>14:14
Chipacafacubatista: charmcraft#150 now has tests, please re-review14:15
mupPR charmcraft#150: when failing to create hard link attempt to use shutil.copy2 <Created by matuskosut> <https://github.com/canonical/charmcraft/pull/150>14:15
facubatistaChipaca, there's no "this PR depends on this other one" so it fixes the diffs, as we have in LP, right?14:16
Chipacafacubatista: correct14:16
facubatistasilly github14:17
Chipacafacubatista: you can get diffs between any two branches though, so that helps14:17
Chipacarelated branches*14:17
Chipacayou can't diff https://github.com/torvalds/linux and https://github.com/python/cpython14:18
facubatistamakes sense :)14:22
* facubatista wants now to have a https://github.com/batista/facundox14:23
facubatistaChipaca, need to improve tests in charmcraft#15014:23
mupPR 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
Chipacayup, you're of course correct14:23
facubatistabthomas, can I get also a review on https://github.com/canonical/charmcraft/pull/155 ? thanks!14:29
mupPR charmcraft#155: Produce the default help text <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/155>14:29
facubatistabthomas, Chipaca, the rest of the builders, thanks for the review! https://github.com/canonical/charmcraft/pull/15617:24
mupPR charmcraft#156: Text builders for simple usage, detailed help, and command help <Created by facundobatista> <https://github.com/canonical/charmcraft/pull/156>17:24
Chipacafacubatista: ack17:26
Chipacafacubatista: 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
mupPR charmcraft#150: when failing to create hard link attempt to use shutil.copy2 <Created by matuskosut> <https://github.com/canonical/charmcraft/pull/150>17:28
Chipacafacubatista: 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), but17:29
facubatistaChipaca, but current code also checks permissions when relevant, right?17:30
Chipacafacubatista: the test_build_generics_tree at least just checks for existence, not that they're the same17:31
Chipacatest_build_generics_simple_files checks that they're the same inode, which i hadn't noticed17:31
facubatistaright, if it's the same inode everything is the same17:32
Chipacaand test_build_generics_simple_dir checks the st_mode17:32
facubatistamy point is that if we copy stuff and remove it's exec bit (for example) we may be breaking the charm17:32
Chipacafacubatista: as i say, i don't disagree that we need to check this stuff :)17:33
Chipacain fact, re-review plz :)17:33
Chipacafacubatista: two files can have the same inode and be different though 🙂17:34
* Chipaca is being suuuuper picky and silly tbh17:34
Chipacafacubatista: (because you check st_ino and not st_dev)17:34
facubatistaChipaca, oh, that's another reason for the `os.link` to fail, right?17:35
facubatistaI mean, fallbacking to copying the file is good for that multiple-devices case17:35
facubatistathat would be rare, though, people mounting other devices inside the charm structure17:36
Chipacafacubatista: that would be an OSError and not a PermissionError though, but yes17:37
Chipacafacubatista: made tricky by PermissionError being an OSError17:37
Chipacait's an OSError with errno==1817:38
Chipacathe xlink one i mean17:38
Chipacafacubatista: i could see somebody trying to mount an outside volume into multipass' build/ for ex17:39
Chipacafacubatista: i'll add a test and a little bit of code to handle it just in case17:40
facubatistaChipaca, ack17:42
Chipacafacubatista: the st_atime check sometimes fails :-/17:47
facubatistaChipaca, "times" are important?17:48
Chipacai can't think of a case of the atime to be important17:49
Chipaca(or the same but in english)17:49
Chipacaand of course now i can't reproduce the failure17:50
facubatistathe trap of testing times :p17:50
Chipacafacubatista: pushed one more commit, hth17:51
* Chipaca gets dinner in th'oven17:51
* facubatista -> tennis18:08
* facubatista is back20:52
mupPR operator#403 opened: Make Harness load default values from config.yaml <Created by johnsca> <https://github.com/canonical/operator/pull/403>21:57
hloeungfacubatista: thanks for your review, updated MP with fixes!22:52
facubatistahloeung, thank you!22:53
hloeungfacubatista: oh hai, when you have a moment, can you go over and re-review / approve? Thanks22:56
facubatistahloeung, I will, tomorrow22:56
facubatistaI'm eoding right now (well, after your review, it was)22:57
hloeungyep yep, works for me22:57

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!