| 14 Aug 2025 |
 emily | I guess I can just duplicate, move, rebase, but ugh | 17:22:01 |
 Charles | 🪦 | 17:22:48 |
 jade_ | yeah. idk. i find so many bugs in code review of stuff that got accepted by cppnix that i think that most code has to be rewritten anyway. i think it's possible to rename the directory in a commit that you can base the cherry-picked one on, or something like that. | 17:25:50 |
| jade_ | 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 | 17:27:16 |
| emily | I don't think your review is right | 17:28:07 |
| emily | because that is a float | 17:28:12 |
| emily | the whole problem is that it is going via floats, so it has float limits | 17:28:26 |
 Rutile (Commentator2.0) feel free to ping | In reply to @emilazy:matrix.org
you can't --accept-tests a change from an eval failure to success or vice versa You should be able to iirc, if that actually doesnt work, create an issue and ill make it work | 17:28:43 |
| Rutile (Commentator2.0) feel free to ping | Oh wsit | 17:28:58 |
| Rutile (Commentator2.0) feel free to ping | Yeah no | 17:29:01 |
| Rutile (Commentator2.0) feel free to ping | Nvm is a different runner not just different output | 17:29:14 |
| 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 |
