)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"80acfd46f33ffb9aec52cfb8989269033a656570","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"feat(tvix/castore/directory): add ClosureValidator"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This can be used to validate a Directory closure (graph of connected"},{"line_number":10,"context_line":"Directories), and their insertion order."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Directories need to be inserted (via `add`), in an order from the leaves"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"93b48fd0_0c3efa5d","line":9,"updated":"2024-03-22 03:35:25.000000000","message":"usually a directory closure is a tree (or a forest, but that does not matter, that\u0027s a set of trees) to me\nare you saying that the directory closure can be **cyclical** connected graph?\n\nin that case, please mention the presence of cycles explicitly.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"0b02e5cd7d42f7252e6d443da38cbe9a7c99e5a4","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"feat(tvix/castore/directory): add ClosureValidator"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This can be used to validate a Directory closure (graph of connected"},{"line_number":10,"context_line":"Directories), and their insertion order."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Directories need to be inserted (via `add`), in an order from the leaves"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"af1ecd2e_60d9e870","line":9,"in_reply_to":"70638533_0f52d3f4","updated":"2024-03-23 14:36:35.000000000","message":"Well, writing a quine could be possible, we didn\u0027t rule out this possibility, di we? But that\u0027s beyond the topic. If we don\u0027t want cyclic stuff, let\u0027s write \"connected DAG\".\n\nNow, it becomes clear for everyone.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"522eb29772f87b818c589f79d0f3a10d7aafe795","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"feat(tvix/castore/directory): add ClosureValidator"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This can be used to validate a Directory closure (graph of connected"},{"line_number":10,"context_line":"Directories), and their insertion order."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Directories need to be inserted (via `add`), in an order from the leaves"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"70638533_0f52d3f4","line":9,"in_reply_to":"93b48fd0_0c3efa5d","updated":"2024-03-22 13:11:31.000000000","message":"How do you produce a cycle? You\u0027d need to bake the hash of your own representation. It\u0027s not a tree either, as a node can have multiple parents.\n\nIt\u0027s a acyclical graph, but the fact that it\u0027s acyclical is kinda obvious due to the whole model being documented as a Merkle structure.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d33c33bc31f30034a59d5e5859f092573f6451ea","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"feat(tvix/castore/directory): add ClosureValidator"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This can be used to validate a Directory closure (graph of connected"},{"line_number":10,"context_line":"Directories), and their insertion order."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Directories need to be inserted (via `add`), in an order from the leaves"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"0d3fd7f8_02445292","line":9,"in_reply_to":"af1ecd2e_60d9e870","updated":"2024-03-23 21:52:31.000000000","message":"Done","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"80acfd46f33ffb9aec52cfb8989269033a656570","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This can be used to validate a Directory closure (graph of connected"},{"line_number":10,"context_line":"Directories), and their insertion order."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Directories need to be inserted (via `add`), in an order from the leaves"},{"line_number":13,"context_line":"to the root. During insertion, We validate as much as we can at that"},{"line_number":14,"context_line":"time:"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"0fd21ed7_371fbcf3","line":12,"updated":"2024-03-22 03:35:25.000000000","message":"you say graph but not tree and you talk about leaves but leaves doesn\u0027t exist in a graph of connected directories in all generality\n\na \u003c-\u003e b is a graph of connected directories, what is a leaf here? (degenerated definition of leaves)","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"522eb29772f87b818c589f79d0f3a10d7aafe795","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This can be used to validate a Directory closure (graph of connected"},{"line_number":10,"context_line":"Directories), and their insertion order."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Directories need to be inserted (via `add`), in an order from the leaves"},{"line_number":13,"context_line":"to the root. During insertion, We validate as much as we can at that"},{"line_number":14,"context_line":"time:"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ab088ee5_2a6eaa9d","line":12,"in_reply_to":"0fd21ed7_371fbcf3","updated":"2024-03-22 13:11:31.000000000","message":"\u003e A node in a graph is a leaf (or a leaf node) if it has degree one and the only edge associated to the node is either undirected or is directed to it. Hence, in an undirected graph, all degree one nodes are leafs. In a directed graph, each degree one node is either a root or a leaf.\n\nI don\u0027t see how leaf is wrong here as a term?\n\nThe part about the graph needing to be connected is that just with the insertion order checks, nothing prevents you from sending a bunch of nodes (entirely single or small graphs), and then never connecting them to the last graph you send. The traversal at the end makes sure all nodes are reachable from the root.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d33c33bc31f30034a59d5e5859f092573f6451ea","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This can be used to validate a Directory closure (graph of connected"},{"line_number":10,"context_line":"Directories), and their insertion order."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Directories need to be inserted (via `add`), in an order from the leaves"},{"line_number":13,"context_line":"to the root. During insertion, We validate as much as we can at that"},{"line_number":14,"context_line":"time:"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7eca24cd_9cfff3aa","line":12,"in_reply_to":"46bab620_7debbead","updated":"2024-03-23 21:52:31.000000000","message":"I replaced the \"graph of\" with \"DAG of\" in the paragraph above, so this is now OK?","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"44319c5ba54b326db69603169de9a1bf15a13aa4","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This can be used to validate a Directory closure (graph of connected"},{"line_number":10,"context_line":"Directories), and their insertion order."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Directories need to be inserted (via `add`), in an order from the leaves"},{"line_number":13,"context_line":"to the root. During insertion, We validate as much as we can at that"},{"line_number":14,"context_line":"time:"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1609c002_78d49c89","line":12,"in_reply_to":"7eca24cd_9cfff3aa","updated":"2024-03-24 17:39:20.000000000","message":"Yep, all good.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"0b02e5cd7d42f7252e6d443da38cbe9a7c99e5a4","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This can be used to validate a Directory closure (graph of connected"},{"line_number":10,"context_line":"Directories), and their insertion order."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Directories need to be inserted (via `add`), in an order from the leaves"},{"line_number":13,"context_line":"to the root. During insertion, We validate as much as we can at that"},{"line_number":14,"context_line":"time:"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"46bab620_7debbead","line":12,"in_reply_to":"ab088ee5_2a6eaa9d","updated":"2024-03-23 14:36:35.000000000","message":"I gave an example of a graph of connected directories where leaves don\u0027t make sense.\n\nIf we say leaves in a (connected) DAG, that\u0027s completely fine, it makes sense with the definition you provided.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"80acfd46f33ffb9aec52cfb8989269033a656570","unresolved":true,"context_lines":[{"line_number":25,"context_line":"from-leaves-to-root order (to be stored somewhere)."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"While assembling that list, a check for graph connectivity is performed"},{"line_number":28,"context_line":"too, to ensure there\u0027s no disconnected graphs, and only one root."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"It adds a test suite for these cases, which is much nicer to test than"},{"line_number":31,"context_line":"where we previously had these checks (only in the gRPC server wrapper)."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"15eaf8ac_b2c42ef3","line":28,"updated":"2024-03-22 03:35:25.000000000","message":"disconnected components, not graphs\n\nyou are looking at all connected components of the potentially non-connected global graph, which is not the same object as we were talking about above","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"522eb29772f87b818c589f79d0f3a10d7aafe795","unresolved":true,"context_lines":[{"line_number":25,"context_line":"from-leaves-to-root order (to be stored somewhere)."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"While assembling that list, a check for graph connectivity is performed"},{"line_number":28,"context_line":"too, to ensure there\u0027s no disconnected graphs, and only one root."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"It adds a test suite for these cases, which is much nicer to test than"},{"line_number":31,"context_line":"where we previously had these checks (only in the gRPC server wrapper)."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9ca6ebca_457f743f","line":28,"in_reply_to":"15eaf8ac_b2c42ef3","updated":"2024-03-22 13:11:31.000000000","message":"I think the proper term would be disconnected subgraph?","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"44319c5ba54b326db69603169de9a1bf15a13aa4","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from-leaves-to-root order (to be stored somewhere)."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"While assembling that list, a check for graph connectivity is performed"},{"line_number":28,"context_line":"too, to ensure there\u0027s no disconnected graphs, and only one root."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"It adds a test suite for these cases, which is much nicer to test than"},{"line_number":31,"context_line":"where we previously had these checks (only in the gRPC server wrapper)."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"d9092f6b_3f5c94fb","line":28,"in_reply_to":"3bbe1e2b_2e9fab05","updated":"2024-03-24 17:39:20.000000000","message":"Yep, all good.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d33c33bc31f30034a59d5e5859f092573f6451ea","unresolved":true,"context_lines":[{"line_number":25,"context_line":"from-leaves-to-root order (to be stored somewhere)."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"While assembling that list, a check for graph connectivity is performed"},{"line_number":28,"context_line":"too, to ensure there\u0027s no disconnected graphs, and only one root."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"It adds a test suite for these cases, which is much nicer to test than"},{"line_number":31,"context_line":"where we previously had these checks (only in the gRPC server wrapper)."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3bbe1e2b_2e9fab05","line":28,"in_reply_to":"609aeeda_5afb8531","updated":"2024-03-23 21:52:31.000000000","message":"Reworded this to\n\n\u003eWhile assembling that list, a check for graph connectivity is performed\n\u003e too, to ensure there\u0027s no separate components being sent (and only one\n\u003e root).\n\nLet me know if you\u0027re OK with this.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"0b02e5cd7d42f7252e6d443da38cbe9a7c99e5a4","unresolved":true,"context_lines":[{"line_number":25,"context_line":"from-leaves-to-root order (to be stored somewhere)."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"While assembling that list, a check for graph connectivity is performed"},{"line_number":28,"context_line":"too, to ensure there\u0027s no disconnected graphs, and only one root."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"It adds a test suite for these cases, which is much nicer to test than"},{"line_number":31,"context_line":"where we previously had these checks (only in the gRPC server wrapper)."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"609aeeda_5afb8531","line":28,"in_reply_to":"9ca6ebca_457f743f","updated":"2024-03-23 14:36:35.000000000","message":"If your main object of interest is a \"connected DAG\", you cannot really talk about the disconnected subgraph of the connected DAG.\nOr you need to talk about the DAG and talk about their connected subgraphs if you want.\n\nEither way: https://en.wikipedia.org/wiki/Component_(graph_theory) is also the valid term here.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"80acfd46f33ffb9aec52cfb8989269033a656570","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0713e2d0_8cc55fef","updated":"2024-03-22 03:35:25.000000000","message":"I mostly have vocabulary problems with the CL, no fundamental issues with the code.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"}],"tvix/castore/src/directoryservice/memory_putter.rs":[{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"5eb5bce5086417198024c579d86e26d4a762fd18","unresolved":true,"context_lines":[{"line_number":39,"context_line":"impl DirectoryClosureValidator {"},{"line_number":40,"context_line":"    /// Insert a new Directory into the closure."},{"line_number":41,"context_line":"    /// Perform individual Directory validation, validation of insertion order"},{"line_number":42,"context_line":"    // and size fields."},{"line_number":43,"context_line":"    #[instrument(level \u003d \"trace\", skip_all, fields(directory.digest\u003d%directory.digest()), err)]"},{"line_number":44,"context_line":"    pub fn add(\u0026mut self, directory: proto::Directory) -\u003e Result\u003c(), Error\u003e {"},{"line_number":45,"context_line":"        let digest \u003d directory.digest();"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"0ed36be2_860eed46","line":42,"updated":"2024-03-20 20:27:08.000000000","message":"```suggestion\n    /// and size fields.\n```","commit_id":"426c6e5eeb381abad2e577681bb8ef94e9a27b42"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"132d2d9796d4bf9e5cbeae63bcf1708a197badb7","unresolved":false,"context_lines":[{"line_number":39,"context_line":"impl DirectoryClosureValidator {"},{"line_number":40,"context_line":"    /// Insert a new Directory into the closure."},{"line_number":41,"context_line":"    /// Perform individual Directory validation, validation of insertion order"},{"line_number":42,"context_line":"    // and size fields."},{"line_number":43,"context_line":"    #[instrument(level \u003d \"trace\", skip_all, fields(directory.digest\u003d%directory.digest()), err)]"},{"line_number":44,"context_line":"    pub fn add(\u0026mut self, directory: proto::Directory) -\u003e Result\u003c(), Error\u003e {"},{"line_number":45,"context_line":"        let digest \u003d directory.digest();"}],"source_content_type":"text/x-rustsrc","patch_set":1,"id":"037f5316_d5b97568","line":42,"in_reply_to":"0ed36be2_860eed46","updated":"2024-03-20 21:12:39.000000000","message":"Done","commit_id":"426c6e5eeb381abad2e577681bb8ef94e9a27b42"}],"tvix/castore/src/directoryservice/validate_closure.rs":[{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"4a1d313c5dbadf92eabccb06d4011fd3dfbf0133","unresolved":true,"context_lines":[{"line_number":93,"context_line":"    /// order."},{"line_number":94,"context_line":"    /// In case no elements have been inserted, returns an empty list."},{"line_number":95,"context_line":"    #[instrument(level \u003d \"trace\", skip_all, err)]"},{"line_number":96,"context_line":"    pub(crate) fn drain(mut self) -\u003e Result\u003cVec\u003cDirectory\u003e, Error\u003e {"},{"line_number":97,"context_line":"        if self.last_directory_digest.is_none() {"},{"line_number":98,"context_line":"            return Ok(vec![]);"},{"line_number":99,"context_line":"        }"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"b2d29465_704a9a5d","line":96,"updated":"2024-03-20 21:13:40.000000000","message":"Not sure about this name. Finalize?","commit_id":"8942bfedc2999a4eb5b13015a002e205ff649f20"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d33c33bc31f30034a59d5e5859f092573f6451ea","unresolved":false,"context_lines":[{"line_number":93,"context_line":"    /// order."},{"line_number":94,"context_line":"    /// In case no elements have been inserted, returns an empty list."},{"line_number":95,"context_line":"    #[instrument(level \u003d \"trace\", skip_all, err)]"},{"line_number":96,"context_line":"    pub(crate) fn drain(mut self) -\u003e Result\u003cVec\u003cDirectory\u003e, Error\u003e {"},{"line_number":97,"context_line":"        if self.last_directory_digest.is_none() {"},{"line_number":98,"context_line":"            return Ok(vec![]);"},{"line_number":99,"context_line":"        }"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"2c1e81de_cab66750","line":96,"in_reply_to":"96422197_4e0ef143","updated":"2024-03-23 21:52:31.000000000","message":"Updated to finalize.","commit_id":"8942bfedc2999a4eb5b13015a002e205ff649f20"},{"author":{"_account_id":1000085,"name":"Connor Brewster","display_name":"cbrewster","email":"cbrewster@hey.com","username":"cbrewster"},"change_message_id":"e217fccfa93c5141c0b7ca7cb95735c0cb3dec90","unresolved":true,"context_lines":[{"line_number":93,"context_line":"    /// order."},{"line_number":94,"context_line":"    /// In case no elements have been inserted, returns an empty list."},{"line_number":95,"context_line":"    #[instrument(level \u003d \"trace\", skip_all, err)]"},{"line_number":96,"context_line":"    pub(crate) fn drain(mut self) -\u003e Result\u003cVec\u003cDirectory\u003e, Error\u003e {"},{"line_number":97,"context_line":"        if self.last_directory_digest.is_none() {"},{"line_number":98,"context_line":"            return Ok(vec![]);"},{"line_number":99,"context_line":"        }"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"96422197_4e0ef143","line":96,"in_reply_to":"b2d29465_704a9a5d","updated":"2024-03-20 22:28:19.000000000","message":"I like finalize more than drain","commit_id":"8942bfedc2999a4eb5b13015a002e205ff649f20"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"80acfd46f33ffb9aec52cfb8989269033a656570","unresolved":true,"context_lines":[{"line_number":106,"context_line":"        // The list of directories we know we need to visit."},{"line_number":107,"context_line":"        // Once we\u0027re finished working, and (there\u0027s no errors), this in reversed order will"},{"line_number":108,"context_line":"        // be a valid insertion order, and directories_and_sizes will be empty."},{"line_number":109,"context_line":"        let mut worklist \u003d Vec::from([self"},{"line_number":110,"context_line":"            .directories_and_sizes"},{"line_number":111,"context_line":"            .remove(\u0026root_digest)"},{"line_number":112,"context_line":"            .expect(\"root digest not found\")"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"8271f977_a52e7000","line":109,"updated":"2024-03-22 03:35:25.000000000","message":"nit: `dir_to_visit`.\n`worklist` is very generic.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"522eb29772f87b818c589f79d0f3a10d7aafe795","unresolved":true,"context_lines":[{"line_number":106,"context_line":"        // The list of directories we know we need to visit."},{"line_number":107,"context_line":"        // Once we\u0027re finished working, and (there\u0027s no errors), this in reversed order will"},{"line_number":108,"context_line":"        // be a valid insertion order, and directories_and_sizes will be empty."},{"line_number":109,"context_line":"        let mut worklist \u003d Vec::from([self"},{"line_number":110,"context_line":"            .directories_and_sizes"},{"line_number":111,"context_line":"            .remove(\u0026root_digest)"},{"line_number":112,"context_line":"            .expect(\"root digest not found\")"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"d79de7d1_2b316ebf","line":109,"in_reply_to":"8271f977_a52e7000","updated":"2024-03-22 13:11:31.000000000","message":"The problem is that this is then also returned, I\u0027m also not happy with `dir_to_visit`. We can just call it `nodes`, and update the comment a bit to say what it represents first?","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d33c33bc31f30034a59d5e5859f092573f6451ea","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        // The list of directories we know we need to visit."},{"line_number":107,"context_line":"        // Once we\u0027re finished working, and (there\u0027s no errors), this in reversed order will"},{"line_number":108,"context_line":"        // be a valid insertion order, and directories_and_sizes will be empty."},{"line_number":109,"context_line":"        let mut worklist \u003d Vec::from([self"},{"line_number":110,"context_line":"            .directories_and_sizes"},{"line_number":111,"context_line":"            .remove(\u0026root_digest)"},{"line_number":112,"context_line":"            .expect(\"root digest not found\")"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"e449d3a0_a5f7ccd8","line":109,"in_reply_to":"a863954c_9acbb7f6","updated":"2024-03-23 21:52:31.000000000","message":"Done","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"0b02e5cd7d42f7252e6d443da38cbe9a7c99e5a4","unresolved":true,"context_lines":[{"line_number":106,"context_line":"        // The list of directories we know we need to visit."},{"line_number":107,"context_line":"        // Once we\u0027re finished working, and (there\u0027s no errors), this in reversed order will"},{"line_number":108,"context_line":"        // be a valid insertion order, and directories_and_sizes will be empty."},{"line_number":109,"context_line":"        let mut worklist \u003d Vec::from([self"},{"line_number":110,"context_line":"            .directories_and_sizes"},{"line_number":111,"context_line":"            .remove(\u0026root_digest)"},{"line_number":112,"context_line":"            .expect(\"root digest not found\")"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"a863954c_9acbb7f6","line":109,"in_reply_to":"d79de7d1_2b316ebf","updated":"2024-03-23 14:36:35.000000000","message":"You can rebind if you want to rename at the end so that the return value intent is clearer, but during the algorithm, it\u0027s clearly a DFS and thus this is a \"to visit directories\" list.\n\nIf you call it nodes, you make it again very generic and you do not say this is a DFS over the nodes of directories, if this is made clearer in a comment, naming it nodes is OK to me.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"80acfd46f33ffb9aec52cfb8989269033a656570","unresolved":true,"context_lines":[{"line_number":112,"context_line":"            .expect(\"root digest not found\")"},{"line_number":113,"context_line":"            .0]);"},{"line_number":114,"context_line":"        // The set of digests that are in the worklist."},{"line_number":115,"context_line":"        let mut worklist_digests \u003d HashSet::new();"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        // This loop moves gradually to the end of worklist, while it\u0027s being"},{"line_number":118,"context_line":"        // extended."}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"73d28c76_1a5abd67","line":115,"updated":"2024-03-22 03:35:25.000000000","message":"nit: `seen_worklist_digests`, better: `seen_dir_digests`.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"522eb29772f87b818c589f79d0f3a10d7aafe795","unresolved":true,"context_lines":[{"line_number":112,"context_line":"            .expect(\"root digest not found\")"},{"line_number":113,"context_line":"            .0]);"},{"line_number":114,"context_line":"        // The set of digests that are in the worklist."},{"line_number":115,"context_line":"        let mut worklist_digests \u003d HashSet::new();"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        // This loop moves gradually to the end of worklist, while it\u0027s being"},{"line_number":118,"context_line":"        // extended."}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"8591c388_c89097bc","line":115,"in_reply_to":"73d28c76_1a5abd67","updated":"2024-03-22 13:11:31.000000000","message":"ack","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d33c33bc31f30034a59d5e5859f092573f6451ea","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            .expect(\"root digest not found\")"},{"line_number":113,"context_line":"            .0]);"},{"line_number":114,"context_line":"        // The set of digests that are in the worklist."},{"line_number":115,"context_line":"        let mut worklist_digests \u003d HashSet::new();"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        // This loop moves gradually to the end of worklist, while it\u0027s being"},{"line_number":118,"context_line":"        // extended."}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"680d9428_3877e08a","line":115,"in_reply_to":"8591c388_c89097bc","updated":"2024-03-23 21:52:31.000000000","message":"Done","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"80acfd46f33ffb9aec52cfb8989269033a656570","unresolved":true,"context_lines":[{"line_number":125,"context_line":"                let child_digest \u003d B3Digest::try_from(child_dir.digest.to_owned()).unwrap(); // validated"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"                // in case the digest is neither already visited nor already on the worklist,"},{"line_number":128,"context_line":"                // enqueue it."},{"line_number":129,"context_line":"                // We don\u0027t need to check for the hash we\u0027re currently"},{"line_number":130,"context_line":"                // visiting, as we can\u0027t self-reference."},{"line_number":131,"context_line":"                if !worklist_digests.contains(\u0026child_digest) {"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"f5df57d4_cb58a048","line":128,"updated":"2024-03-22 03:35:25.000000000","message":"nit: push it back, this is not a LIFO driven algorithm; enqueue should/can mean push_front instead of push_back, I\u0027d argue.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"44319c5ba54b326db69603169de9a1bf15a13aa4","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                let child_digest \u003d B3Digest::try_from(child_dir.digest.to_owned()).unwrap(); // validated"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"                // in case the digest is neither already visited nor already on the worklist,"},{"line_number":128,"context_line":"                // enqueue it."},{"line_number":129,"context_line":"                // We don\u0027t need to check for the hash we\u0027re currently"},{"line_number":130,"context_line":"                // visiting, as we can\u0027t self-reference."},{"line_number":131,"context_line":"                if !worklist_digests.contains(\u0026child_digest) {"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"763d0d66_860e2127","line":128,"in_reply_to":"74818da3_63af666c","updated":"2024-03-24 17:39:20.000000000","message":"All good to me.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d33c33bc31f30034a59d5e5859f092573f6451ea","unresolved":true,"context_lines":[{"line_number":125,"context_line":"                let child_digest \u003d B3Digest::try_from(child_dir.digest.to_owned()).unwrap(); // validated"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"                // in case the digest is neither already visited nor already on the worklist,"},{"line_number":128,"context_line":"                // enqueue it."},{"line_number":129,"context_line":"                // We don\u0027t need to check for the hash we\u0027re currently"},{"line_number":130,"context_line":"                // visiting, as we can\u0027t self-reference."},{"line_number":131,"context_line":"                if !worklist_digests.contains(\u0026child_digest) {"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"74818da3_63af666c","line":128,"in_reply_to":"b8025e43_66e05605","updated":"2024-03-23 21:52:31.000000000","message":"Updated to\n\n```\n                // In case the digest is neither already visited nor already in dirs_to_visit,\n                // add it to the end of it.\n```\n\nwhich feels sounds a bit more natural in this context, and should also not be ambiguous.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"522eb29772f87b818c589f79d0f3a10d7aafe795","unresolved":true,"context_lines":[{"line_number":125,"context_line":"                let child_digest \u003d B3Digest::try_from(child_dir.digest.to_owned()).unwrap(); // validated"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"                // in case the digest is neither already visited nor already on the worklist,"},{"line_number":128,"context_line":"                // enqueue it."},{"line_number":129,"context_line":"                // We don\u0027t need to check for the hash we\u0027re currently"},{"line_number":130,"context_line":"                // visiting, as we can\u0027t self-reference."},{"line_number":131,"context_line":"                if !worklist_digests.contains(\u0026child_digest) {"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"b8025e43_66e05605","line":128,"in_reply_to":"f5df57d4_cb58a048","updated":"2024-03-22 13:11:31.000000000","message":"Acknowledged","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"80acfd46f33ffb9aec52cfb8989269033a656570","unresolved":true,"context_lines":[{"line_number":129,"context_line":"                // We don\u0027t need to check for the hash we\u0027re currently"},{"line_number":130,"context_line":"                // visiting, as we can\u0027t self-reference."},{"line_number":131,"context_line":"                if !worklist_digests.contains(\u0026child_digest) {"},{"line_number":132,"context_line":"                    worklist.push("},{"line_number":133,"context_line":"                        self.directories_and_sizes"},{"line_number":134,"context_line":"                            .remove(\u0026child_digest)"},{"line_number":135,"context_line":"                            .expect(\"child digest not found\")"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"f5d3b9aa_49675478","line":132,"updated":"2024-03-22 03:35:25.000000000","message":"future-work/performance: this will generate a bunch of allocations on large closures but will be amortized on the long run by the classical Vector resize behavior. this can be further amortized w.r.t. constants by passing a hint of directory size in `add`, maintaining it, and using it to `reserve` the `worklist`\u0027s vec.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"d33c33bc31f30034a59d5e5859f092573f6451ea","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                // We don\u0027t need to check for the hash we\u0027re currently"},{"line_number":130,"context_line":"                // visiting, as we can\u0027t self-reference."},{"line_number":131,"context_line":"                if !worklist_digests.contains(\u0026child_digest) {"},{"line_number":132,"context_line":"                    worklist.push("},{"line_number":133,"context_line":"                        self.directories_and_sizes"},{"line_number":134,"context_line":"                            .remove(\u0026child_digest)"},{"line_number":135,"context_line":"                            .expect(\"child digest not found\")"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"bf8cdcdb_0e06ceac","line":132,"in_reply_to":"04d7a20c_c4100d4a","updated":"2024-03-23 21:52:31.000000000","message":"Done","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000073,"name":"raitobezarius","display_name":"Ryan Lahfa","email":"tvl@lahfa.xyz","username":"raitobezarius"},"change_message_id":"0b02e5cd7d42f7252e6d443da38cbe9a7c99e5a4","unresolved":true,"context_lines":[{"line_number":129,"context_line":"                // We don\u0027t need to check for the hash we\u0027re currently"},{"line_number":130,"context_line":"                // visiting, as we can\u0027t self-reference."},{"line_number":131,"context_line":"                if !worklist_digests.contains(\u0026child_digest) {"},{"line_number":132,"context_line":"                    worklist.push("},{"line_number":133,"context_line":"                        self.directories_and_sizes"},{"line_number":134,"context_line":"                            .remove(\u0026child_digest)"},{"line_number":135,"context_line":"                            .expect(\"child digest not found\")"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"04d7a20c_c4100d4a","line":132,"in_reply_to":"8749313d_0732cf7c","updated":"2024-03-23 14:36:35.000000000","message":"Nice.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"522eb29772f87b818c589f79d0f3a10d7aafe795","unresolved":true,"context_lines":[{"line_number":129,"context_line":"                // We don\u0027t need to check for the hash we\u0027re currently"},{"line_number":130,"context_line":"                // visiting, as we can\u0027t self-reference."},{"line_number":131,"context_line":"                if !worklist_digests.contains(\u0026child_digest) {"},{"line_number":132,"context_line":"                    worklist.push("},{"line_number":133,"context_line":"                        self.directories_and_sizes"},{"line_number":134,"context_line":"                            .remove(\u0026child_digest)"},{"line_number":135,"context_line":"                            .expect(\"child digest not found\")"}],"source_content_type":"text/x-rustsrc","patch_set":4,"id":"8749313d_0732cf7c","line":132,"in_reply_to":"f5d3b9aa_49675478","updated":"2024-03-22 13:11:31.000000000","message":"I think we should initialize `worklist` with a capaciy set to the number of elements in `directories_and_sizes`, yes. Good catch.","commit_id":"32e1f732476325fd6e65ad25e6caeb522ce197a9"}]}