| 14 Aug 2025 |
 jade_ | oh yes, you're right. i hate implicit conversions (code style issue in this code imo) | 17:30:55 |
 emily | I'm just saying, missing things in code review is a general phenomena :p | 17:31:54 |
 Sergei Zimmerman (xokdvium) | In reply to @jade_:matrix.org
for example, i found straight up wrong code in here (in addition to error messages that refer to concepts that only exist in C++ and not in user facing places)
i am still happy to port the fix, but it's going to experience some rewrites FWIW we are running UBSAN in CI for some time now, so signed overflow should be caught (modulo compiler quirks) | 17:51:49 |
| Sergei Zimmerman (xokdvium) | Test coverage is another matter though | 17:52:56 |
| Sergei Zimmerman (xokdvium) | Does Lix track code coverage in CI? | 17:54:32 |
| Sergei Zimmerman (xokdvium) | (Not a panacea but quite useful) | 17:55:05 |
| jade_ | I don't think we have tracking of it per se because our tests run in nix, but we probably could, in principle. | 18:13:31 |
| Sergei Zimmerman (xokdvium) | I've recently added clang-based source coverage to CI for nix. | 18:13:54 |
| Sergei Zimmerman (xokdvium) | (And the tests run in nix as well) | 18:14:08 |
| jade_ | it probably needs to consider coverage of the functional tests as well, so that's a little bit fun | 18:14:25 |
| Sergei Zimmerman (xokdvium) | Yup, functional tests are included as well. The only thing that's missing is the nixos tests | 18:14:45 |
| Sergei Zimmerman (xokdvium) | It's a bit undercooked at the moment with some rough edges, but at least it works and enough to track the metrics and push html to hydra | 18:15:44 |
| Sergei Zimmerman (xokdvium) | https://github.com/NixOS/nix/blob/58eabe7479177dc63cf3c704230a45b9260b3fec/ci/gha/tests/default.nix#L172 | 18:16:22 |
| jade_ | so umm. they seem to have defined this as "throws .. sometimes .. if certain int to float conversions fail .. sometimes" | 18:17:01 |
| jade_ | surely these should just passthrough integers entirely unmodified | 18:17:12 |
| jade_ | right? | 18:17:18 |
| emily | I assume the conservative throw is for the same reason you did one in fromJSON | 18:17:40 |
| emily | keeps options open | 18:17:47 |
| jade_ | I also rewrote the float to int conversion to use the correct function | 18:17:51 |
| emily | but yeah it's hard to imagine a different sensible behaviour to use here | 18:17:56 |
| emily | but I think if you pass through integers the practical value you get is different to what it was historically | 18:18:18 |
| jade_ | yeah idk, i think converting it to a float first is just .. weird and hard to ensure it behaves consistently, comparatively | 18:18:19 |
| emily | because of how the UB would be compiled in practice | 18:18:21 |
| jade_ | oh | 18:18:24 |
| jade_ | ugh | 18:18:26 |
| jade_ | yeah | 18:18:27 |
| jade_ | okay | 18:18:28 |
| emily | so having a deprecation period where it errors seems sensible | 18:18:34 |
| emily | put up a whole bunch of changes for the TOML stuff (both the port of the Nix change for stable branches and the dropping of the experimental feature for main), NUL bytes in JSON/TOML, and overflowing JSON literals | 19:11:24 |
 Rutile (Commentator2.0) feel free to ping | https://gerrit.lix.systems/c/lix/+/3333/47
can someone punch besadii? | 19:39:52 |
