)]}'
{"tvix/cli/src/main.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"72b959ed558cc333498f4554b993f29094838982","unresolved":true,"context_lines":[{"line_number":175,"context_line":"    .add_builtins(impure_builtins())"},{"line_number":176,"context_line":"    .env(env);"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    eval_builder \u003d add_derivation_builtins(eval_builder, Rc::clone(\u0026tvix_store_io));"},{"line_number":179,"context_line":"    eval_builder \u003d add_fetcher_builtins(eval_builder, Rc::clone(\u0026tvix_store_io));"},{"line_number":180,"context_line":"    eval_builder \u003d add_import_builtins(eval_builder, tvix_store_io);"},{"line_number":181,"context_line":"    eval_builder \u003d configure_nix_path(eval_builder, \u0026args.nix_search_path);"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"8b056bb0_5672a0ac","line":178,"updated":"2024-07-06 11:48:27.000000000","message":"Are you happy with keeping this pattern, of something else taking the builder and then adding things to it?\n\nI feel like I\u0027d rather see these `add_*_builtins` functions to be refactored to return a `impl IntoIterator\u003cItem \u003d (\u0026\u0027static str, Value)\u003e` that can then be fed to `add_builtins` here.\n\nOf course, in a followup, but still worthy to have this conversation here.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000010,"name":"aspen","email":"root@gws.fyi","username":"aspen"},"change_message_id":"5c37c68099460375b9dcf31631e5d77659460886","unresolved":false,"context_lines":[{"line_number":175,"context_line":"    .add_builtins(impure_builtins())"},{"line_number":176,"context_line":"    .env(env);"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    eval_builder \u003d add_derivation_builtins(eval_builder, Rc::clone(\u0026tvix_store_io));"},{"line_number":179,"context_line":"    eval_builder \u003d add_fetcher_builtins(eval_builder, Rc::clone(\u0026tvix_store_io));"},{"line_number":180,"context_line":"    eval_builder \u003d add_import_builtins(eval_builder, tvix_store_io);"},{"line_number":181,"context_line":"    eval_builder \u003d configure_nix_path(eval_builder, \u0026args.nix_search_path);"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"9c05efb5_fb6af0d3","line":178,"in_reply_to":"8b056bb0_5672a0ac","updated":"2024-07-06 13:38:54.000000000","message":"No, I don\u0027t love this tbh. I\u0027ll change it in a follow-up for sure.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"}],"tvix/eval/src/lib.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"b64ffc2b2b0c5fde233c0ed9cc5320b6136717d1","unresolved":true,"context_lines":[{"line_number":153,"context_line":"        self.with_enable_import(true)"},{"line_number":154,"context_line":"    }"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    pub fn builtins_mut(\u0026mut self) -\u003e \u0026mut Vec\u003c(\u0026\u0027static str, Value)\u003e {"},{"line_number":157,"context_line":"        \u0026mut self.builtins"},{"line_number":158,"context_line":"    }"},{"line_number":159,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"fb8a3395_2b9437e4","line":156,"updated":"2024-07-06 12:30:07.000000000","message":"This isn\u0027t really used anywhere (yet), and if we go with the approach suggested in https://cl.tvl.fyi/c/depot/+/11956/comment/8b056bb0_5672a0ac/, there should be little reason to provide this, so I\u0027d prefer removing it.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000010,"name":"aspen","email":"root@gws.fyi","username":"aspen"},"change_message_id":"5c37c68099460375b9dcf31631e5d77659460886","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        self.with_enable_import(true)"},{"line_number":154,"context_line":"    }"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    pub fn builtins_mut(\u0026mut self) -\u003e \u0026mut Vec\u003c(\u0026\u0027static str, Value)\u003e {"},{"line_number":157,"context_line":"        \u0026mut self.builtins"},{"line_number":158,"context_line":"    }"},{"line_number":159,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"1c8cdf1d_3b03d66a","line":156,"in_reply_to":"fb8a3395_2b9437e4","updated":"2024-07-06 13:38:54.000000000","message":"yeah, agreed. Done.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"b64ffc2b2b0c5fde233c0ed9cc5320b6136717d1","unresolved":true,"context_lines":[{"line_number":169,"context_line":"        Self { strict, ..self }"},{"line_number":170,"context_line":"    }"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    pub fn strict(self) -\u003e Self {"},{"line_number":173,"context_line":"        self.with_strict(true)"},{"line_number":174,"context_line":"    }"},{"line_number":175,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"78246115_512239ef","line":172,"updated":"2024-07-06 12:30:07.000000000","message":"We should only have one variant of these, not both `[with_]foo` builder methods.\n\nI\u0027d probably keep the `with_` for setters to make it clear (so `with_import(bool)`, `with_io_handle`, `with_compiler_observer`, …), and use `add_` explicitly for places that don\u0027t override (`add_src_builtin`, `add_builtins`).","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"613f7ce1c652b33a135fe77a59b44096e197a81a","unresolved":false,"context_lines":[{"line_number":169,"context_line":"        Self { strict, ..self }"},{"line_number":170,"context_line":"    }"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    pub fn strict(self) -\u003e Self {"},{"line_number":173,"context_line":"        self.with_strict(true)"},{"line_number":174,"context_line":"    }"},{"line_number":175,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"a98b8db1_2ac8d8b7","line":172,"in_reply_to":"2fd6e6e1_c0f90c19","updated":"2024-07-06 14:45:40.000000000","message":"I think we can revisit this as part of https://cl.tvl.fyi/c/depot/+/11956/comment/4c6198cd_f4a17e6b/.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000010,"name":"aspen","email":"root@gws.fyi","username":"aspen"},"change_message_id":"5c37c68099460375b9dcf31631e5d77659460886","unresolved":true,"context_lines":[{"line_number":169,"context_line":"        Self { strict, ..self }"},{"line_number":170,"context_line":"    }"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    pub fn strict(self) -\u003e Self {"},{"line_number":173,"context_line":"        self.with_strict(true)"},{"line_number":174,"context_line":"    }"},{"line_number":175,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"2fd6e6e1_c0f90c19","line":172,"in_reply_to":"78246115_512239ef","updated":"2024-07-06 13:38:54.000000000","message":"I soft-disagree, actually - I quite like having a more \"maximalist\" interface to builders, since the intent is to be used as a \"nice\" API for users.\n\nFeel free to push back if you feel more strongly about it than I do, though.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"72b959ed558cc333498f4554b993f29094838982","unresolved":true,"context_lines":[{"line_number":224,"context_line":"impl\u003c\u0027co, \u0027ro, \u0027env\u003e EvaluationBuilder\u003c\u0027co, \u0027ro, \u0027env, Box\u003cdyn EvalIO\u003e\u003e {"},{"line_number":225,"context_line":"    /// Initialize an `Evaluation`, without the import statement available, and"},{"line_number":226,"context_line":"    /// all IO operations stubbed out."},{"line_number":227,"context_line":"    pub fn new_pure() -\u003e Self {"},{"line_number":228,"context_line":"        Self::new(Box::new(DummyIO) as Box\u003cdyn EvalIO\u003e).with_enable_import(false)"},{"line_number":229,"context_line":"    }"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"4c6198cd_f4a17e6b","line":227,"updated":"2024-07-06 11:48:27.000000000","message":"I\u0027d prefer explicitly asking for an IO impl in the constructor (and maybe have a `Default` impl using that with `DummyIO`), and having an `enable_import()` modifier to flip that boolean.\n\nThis whole pure/impure distinction is a bit misleading, also see https://b.tvl.fyi/issues/404.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"62dc44638ad1642736021c405c2d4e55248c7d7b","unresolved":true,"context_lines":[{"line_number":224,"context_line":"impl\u003c\u0027co, \u0027ro, \u0027env\u003e EvaluationBuilder\u003c\u0027co, \u0027ro, \u0027env, Box\u003cdyn EvalIO\u003e\u003e {"},{"line_number":225,"context_line":"    /// Initialize an `Evaluation`, without the import statement available, and"},{"line_number":226,"context_line":"    /// all IO operations stubbed out."},{"line_number":227,"context_line":"    pub fn new_pure() -\u003e Self {"},{"line_number":228,"context_line":"        Self::new(Box::new(DummyIO) as Box\u003cdyn EvalIO\u003e).with_enable_import(false)"},{"line_number":229,"context_line":"    }"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"df4b395c_8f5fc70e","line":227,"in_reply_to":"4c6198cd_f4a17e6b","updated":"2024-07-06 11:51:21.000000000","message":"(ah yes, and the operations doing actual filesystem IO should probably just be feature-flagged on `target_family \u003d \"unix\"` to make them not be pulled in in the wasm case)","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000010,"name":"aspen","email":"root@gws.fyi","username":"aspen"},"change_message_id":"e2f6ce38856b8e4c69f9d5524d68645104fae9e2","unresolved":false,"context_lines":[{"line_number":224,"context_line":"impl\u003c\u0027co, \u0027ro, \u0027env\u003e EvaluationBuilder\u003c\u0027co, \u0027ro, \u0027env, Box\u003cdyn EvalIO\u003e\u003e {"},{"line_number":225,"context_line":"    /// Initialize an `Evaluation`, without the import statement available, and"},{"line_number":226,"context_line":"    /// all IO operations stubbed out."},{"line_number":227,"context_line":"    pub fn new_pure() -\u003e Self {"},{"line_number":228,"context_line":"        Self::new(Box::new(DummyIO) as Box\u003cdyn EvalIO\u003e).with_enable_import(false)"},{"line_number":229,"context_line":"    }"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"5f647f74_be2d5bfc","line":227,"in_reply_to":"639069cc_601ad76f","updated":"2024-07-06 14:47:32.000000000","message":"definitely.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"613f7ce1c652b33a135fe77a59b44096e197a81a","unresolved":false,"context_lines":[{"line_number":224,"context_line":"impl\u003c\u0027co, \u0027ro, \u0027env\u003e EvaluationBuilder\u003c\u0027co, \u0027ro, \u0027env, Box\u003cdyn EvalIO\u003e\u003e {"},{"line_number":225,"context_line":"    /// Initialize an `Evaluation`, without the import statement available, and"},{"line_number":226,"context_line":"    /// all IO operations stubbed out."},{"line_number":227,"context_line":"    pub fn new_pure() -\u003e Self {"},{"line_number":228,"context_line":"        Self::new(Box::new(DummyIO) as Box\u003cdyn EvalIO\u003e).with_enable_import(false)"},{"line_number":229,"context_line":"    }"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"639069cc_601ad76f","line":227,"in_reply_to":"d3dc2d26_39b9e8d7","updated":"2024-07-06 14:45:40.000000000","message":"sure, just wanted to make sure we agree on the builder API still being in flux for the time being.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"},{"author":{"_account_id":1000010,"name":"aspen","email":"root@gws.fyi","username":"aspen"},"change_message_id":"5c37c68099460375b9dcf31631e5d77659460886","unresolved":true,"context_lines":[{"line_number":224,"context_line":"impl\u003c\u0027co, \u0027ro, \u0027env\u003e EvaluationBuilder\u003c\u0027co, \u0027ro, \u0027env, Box\u003cdyn EvalIO\u003e\u003e {"},{"line_number":225,"context_line":"    /// Initialize an `Evaluation`, without the import statement available, and"},{"line_number":226,"context_line":"    /// all IO operations stubbed out."},{"line_number":227,"context_line":"    pub fn new_pure() -\u003e Self {"},{"line_number":228,"context_line":"        Self::new(Box::new(DummyIO) as Box\u003cdyn EvalIO\u003e).with_enable_import(false)"},{"line_number":229,"context_line":"    }"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"d3dc2d26_39b9e8d7","line":227,"in_reply_to":"df4b395c_8f5fc70e","updated":"2024-07-06 13:38:54.000000000","message":"Would you mind doing this yourself in a follow-up? I think you probably know more about what you think this should look like than me.","commit_id":"e3f94df7305889965d28e7246acacf198bcdb762"}]}