| === xavpaice- is now known as xavpaice | ||
| Chipaca | ｇｏｏｏｏｏｏｏｏｄ ｍｏｒｎｉｎｇ！ | 08:07 |
|---|---|---|
| bthomas | 𝐌𝐨𝐫𝐧𝐢𝐧𝐠 | 08:08 |
| bthomas | 08:08 | |
| bthomas | 08:08 | |
| bthomas | 08:08 | |
| bthomas | hmmmm... unifonter added an extra newline | 08:08 |
| Chipaca | i need to reboot for a bios upgrade apparently | 08:10 |
| Chipaca | s/bios/firmware/ | 08:10 |
| Chipaca | bbiab | 08:10 |
| Chipaca | 𝑤𝑓𝑚 | 08:16 |
| Chipaca | bthomas: ^ no spurious newline | 08:17 |
| bthomas | Chipaca: I do C-u M-! unifonter -k ... | 08:17 |
| bthomas | 𝑡𝑟𝑦𝑖𝑛𝑔 𝑎𝑔𝑎𝑖𝑛 | 08:18 |
| bthomas | 08:18 | |
| 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:20 |
| 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:28 |
| 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 :) | 08:29 |
| Chipaca | jam: morning! | 09:54 |
| * Chipaca steps out for a bit | 10:36 | |
| jam | morning Chipaca | 10:46 |
| facubatista | ¡Muy buenos días a todos! | 10:57 |
| bthomas | नमस्ते facubatista | 11:01 |
| facubatista | hola bthomas | 11:01 |
| bthomas | Morning jam | 11:01 |
| 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:02 |
| 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:12 |
| 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:23 |
| facubatista | bthomas, thanks | 11:28 |
| 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:50 |
| Chipaca | facubatista: I'll merge it now, ok? | 13:54 |
| * Chipaca takes that as a yes | 13:56 | |
| facubatista | Chipaca, yes, thanks | 13:57 |
| 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:14 |
| 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:15 |
| 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:16 |
| 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:17 |
| Chipaca | you can't diff https://github.com/torvalds/linux and https://github.com/python/cpython | 14:18 |
| facubatista | makes sense :) | 14:22 |
| * 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:23 |
| 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:29 |
| bthomas | done | 14:39 |
| 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:24 |
| Chipaca | facubatista: ack | 17:26 |
| 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:28 |
| 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:29 |
| facubatista | Chipaca, but current code also checks permissions when relevant, right? | 17:30 |
| 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:31 |
| 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:32 |
| 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:33 |
| 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:34 |
| 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:35 |
| facubatista | that would be rare, though, people mounting other devices inside the charm structure | 17:36 |
| 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:37 |
| Chipaca | it's an OSError with errno==18 | 17:38 |
| Chipaca | the xlink one i mean | 17:38 |
| Chipaca | facubatista: i could see somebody trying to mount an outside volume into multipass' build/ for ex | 17:39 |
| Chipaca | facubatista: i'll add a test and a little bit of code to handle it just in case | 17:40 |
| facubatista | Chipaca, ack | 17:42 |
| Chipaca | facubatista: the st_atime check sometimes fails :-/ | 17:47 |
| facubatista | Chipaca, "times" are important? | 17:48 |
| Chipaca | i can't think of a case of the atime to be important | 17:49 |
| Chipaca | (or the same but in english) | 17:49 |
| Chipaca | and of course now i can't reproduce the failure | 17:50 |
| facubatista | the trap of testing times :p | 17:50 |
| Chipaca | facubatista: pushed one more commit, hth | 17:51 |
| * Chipaca gets dinner in th'oven | 17:51 | |
| * facubatista -> tennis | 18:08 | |
| * facubatista is back | 20:52 | |
| mup | PR operator#403 opened: Make Harness load default values from config.yaml <Created by johnsca> <https://github.com/canonical/operator/pull/403> | 21:57 |
| hloeung | facubatista: thanks for your review, updated MP with fixes! | 22:52 |
| facubatista | hloeung, thank you! | 22:53 |
| 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:56 |
| facubatista | I'm eoding right now (well, after your review, it was) | 22:57 |
| hloeung | yep yep, works for me | 22:57 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!