)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000001,"name":"tazjin","email":"tazjin@tvl.su","username":"tazjin"},"change_message_id":"36c4fe35b331fd423c373e499c20fb14cec8c3c8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a959b83b_8aebd1b5","updated":"2022-12-27 20:50:38.000000000","message":"Just a suggestion, but maybe add helpers for generating the test fixtures (or constructors in general?)? They are quite verbose otherwise.","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000001,"name":"tazjin","email":"tazjin@tvl.su","username":"tazjin"},"change_message_id":"42d31e299bf9a86ab56efccb1cd6e688b5166530","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7cf9bb39_2e6f5d81","in_reply_to":"8a7b1f7b_3d3811e2","updated":"2022-12-28 10:42:59.000000000","message":"Ack","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"8cb92694f8b0f8008a35c094ef14de8adaba7c98","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8a7b1f7b_3d3811e2","in_reply_to":"a959b83b_8aebd1b5","updated":"2022-12-27 23:18:40.000000000","message":"I used ..Default::default() in most of the occurences, but it didn\u0027t help too much in terms of pure line count.\n\nThe tests in some places intentionally place things into different lists, so it\u0027s hard to write generic constructors. PTAL.","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"}],"tvix/store/src/proto.rs":[{"author":{"_account_id":1000001,"name":"tazjin","email":"tazjin@tvl.su","username":"tazjin"},"change_message_id":"e965ad9382008428f1e9379a1bdcdfeace1399c2","unresolved":true,"context_lines":[{"line_number":36,"context_line":"        hasher.update(\u0026self.encode_to_vec()).finalize().as_bytes()[..].to_vec()"},{"line_number":37,"context_line":"    }"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    // name_is_valid checks a name for validity."},{"line_number":40,"context_line":"    // We disallow slashes, null bytes, \u0027.\u0027, \u0027..\u0027 and the empty string."},{"line_number":41,"context_line":"    // Depending on the context, a *Node message with an empty string as name isValidName"},{"line_number":42,"context_line":"    // allowed, but they don\u0027t occur inside a Directory message."}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"b259aed2_506f621a","line":39,"range":{"start_line":39,"start_character":4,"end_line":39,"end_character":6},"updated":"2022-12-27 20:38:37.000000000","message":"doc comments in rust start with three slashes (`///`)","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"8cb92694f8b0f8008a35c094ef14de8adaba7c98","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        hasher.update(\u0026self.encode_to_vec()).finalize().as_bytes()[..].to_vec()"},{"line_number":37,"context_line":"    }"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    // name_is_valid checks a name for validity."},{"line_number":40,"context_line":"    // We disallow slashes, null bytes, \u0027.\u0027, \u0027..\u0027 and the empty string."},{"line_number":41,"context_line":"    // Depending on the context, a *Node message with an empty string as name isValidName"},{"line_number":42,"context_line":"    // allowed, but they don\u0027t occur inside a Directory message."}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"0f612939_22e7d819","line":39,"range":{"start_line":39,"start_character":4,"end_line":39,"end_character":6},"in_reply_to":"b259aed2_506f621a","updated":"2022-12-27 23:18:40.000000000","message":"Done","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000001,"name":"tazjin","email":"tazjin@tvl.su","username":"tazjin"},"change_message_id":"e965ad9382008428f1e9379a1bdcdfeace1399c2","unresolved":true,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    // name_is_valid checks a name for validity."},{"line_number":40,"context_line":"    // We disallow slashes, null bytes, \u0027.\u0027, \u0027..\u0027 and the empty string."},{"line_number":41,"context_line":"    // Depending on the context, a *Node message with an empty string as name isValidName"},{"line_number":42,"context_line":"    // allowed, but they don\u0027t occur inside a Directory message."},{"line_number":43,"context_line":"    fn name_is_valid(name: \u0026String) -\u003e bool {"},{"line_number":44,"context_line":"        if name \u003d\u003d \"\" || name \u003d\u003d \"..\" || name \u003d\u003d \".\" || name.contains(\"\\x00\") || name.contains(\"/\")"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"25a5cb57_b44870cb","line":41,"range":{"start_line":41,"start_character":35,"end_line":41,"end_character":40},"updated":"2022-12-27 20:38:37.000000000","message":"typo?\n\nEither way, in the doc comments you can auto-link to resolvable identifiers using e.g. `[Node]`","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"8cb92694f8b0f8008a35c094ef14de8adaba7c98","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    // name_is_valid checks a name for validity."},{"line_number":40,"context_line":"    // We disallow slashes, null bytes, \u0027.\u0027, \u0027..\u0027 and the empty string."},{"line_number":41,"context_line":"    // Depending on the context, a *Node message with an empty string as name isValidName"},{"line_number":42,"context_line":"    // allowed, but they don\u0027t occur inside a Directory message."},{"line_number":43,"context_line":"    fn name_is_valid(name: \u0026String) -\u003e bool {"},{"line_number":44,"context_line":"        if name \u003d\u003d \"\" || name \u003d\u003d \"..\" || name \u003d\u003d \".\" || name.contains(\"\\x00\") || name.contains(\"/\")"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"c3a9dc12_9da092c6","line":41,"range":{"start_line":41,"start_character":35,"end_line":41,"end_character":40},"in_reply_to":"25a5cb57_b44870cb","updated":"2022-12-27 23:18:40.000000000","message":"The three lists in a Directory message are (Vecs of) DirectoryNode, FileNode and SymlinkNode, that\u0027s what I meant here. I can also explicitly list (and link) them here.","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000001,"name":"tazjin","email":"tazjin@tvl.su","username":"tazjin"},"change_message_id":"e965ad9382008428f1e9379a1bdcdfeace1399c2","unresolved":true,"context_lines":[{"line_number":40,"context_line":"    // We disallow slashes, null bytes, \u0027.\u0027, \u0027..\u0027 and the empty string."},{"line_number":41,"context_line":"    // Depending on the context, a *Node message with an empty string as name isValidName"},{"line_number":42,"context_line":"    // allowed, but they don\u0027t occur inside a Directory message."},{"line_number":43,"context_line":"    fn name_is_valid(name: \u0026String) -\u003e bool {"},{"line_number":44,"context_line":"        if name \u003d\u003d \"\" || name \u003d\u003d \"..\" || name \u003d\u003d \".\" || name.contains(\"\\x00\") || name.contains(\"/\")"},{"line_number":45,"context_line":"        {"},{"line_number":46,"context_line":"            return false;"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"3c02e8f1_3784dc63","line":43,"range":{"start_line":43,"start_character":39,"end_line":43,"end_character":43},"updated":"2022-12-27 20:38:37.000000000","message":"the usage below is quite verbose, I think it would be better to have this already return a `Result\u003c(), *Error\u003e` so that you can just call `name_is_valid(foo.name)?` below.\n\nAlso, since this function does not take a `Self` or `self` parameter, it should be moved out of the `impl` block (it is not related to the `Directory` type)","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"8cb92694f8b0f8008a35c094ef14de8adaba7c98","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    // We disallow slashes, null bytes, \u0027.\u0027, \u0027..\u0027 and the empty string."},{"line_number":41,"context_line":"    // Depending on the context, a *Node message with an empty string as name isValidName"},{"line_number":42,"context_line":"    // allowed, but they don\u0027t occur inside a Directory message."},{"line_number":43,"context_line":"    fn name_is_valid(name: \u0026String) -\u003e bool {"},{"line_number":44,"context_line":"        if name \u003d\u003d \"\" || name \u003d\u003d \"..\" || name \u003d\u003d \".\" || name.contains(\"\\x00\") || name.contains(\"/\")"},{"line_number":45,"context_line":"        {"},{"line_number":46,"context_line":"            return false;"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"948f709c_04f9b6bb","line":43,"range":{"start_line":43,"start_character":39,"end_line":43,"end_character":43},"in_reply_to":"3c02e8f1_3784dc63","updated":"2022-12-27 23:18:40.000000000","message":"is_valid implies it returns a boolean.\n\nI renamed it to validate_node_name and have it return an Error.\n\nI also factored out the three other checks into similar functions.","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000001,"name":"tazjin","email":"tazjin@tvl.su","username":"tazjin"},"change_message_id":"e1170e403b3eee3d9515d1d39a3f5dc1b70daf45","unresolved":true,"context_lines":[{"line_number":56,"context_line":"    pub fn validate(\u0026self) -\u003e Result\u003c(), ValidateDirectoryError\u003e {"},{"line_number":57,"context_line":"        let mut seen_names \u003d HashSet::new();"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        let mut last_directory_name \u003d \"\".to_string();"},{"line_number":60,"context_line":"        let mut last_file_name \u003d \"\".to_string();"},{"line_number":61,"context_line":"        let mut last_symlink_name \u003d \"\".to_string();"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"3e71626b_b05a4274","line":59,"range":{"start_line":59,"start_character":41,"end_line":59,"end_character":53},"updated":"2022-12-27 20:50:04.000000000","message":"I think these can just be `\u0026mut str` instead of `mut String`, as the lifetime of `\u0026self` outlives the body.","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"8cb92694f8b0f8008a35c094ef14de8adaba7c98","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    pub fn validate(\u0026self) -\u003e Result\u003c(), ValidateDirectoryError\u003e {"},{"line_number":57,"context_line":"        let mut seen_names \u003d HashSet::new();"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        let mut last_directory_name \u003d \"\".to_string();"},{"line_number":60,"context_line":"        let mut last_file_name \u003d \"\".to_string();"},{"line_number":61,"context_line":"        let mut last_symlink_name \u003d \"\".to_string();"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"bec5b85f_505ea4e0","line":59,"range":{"start_line":59,"start_character":41,"end_line":59,"end_character":53},"in_reply_to":"3e71626b_b05a4274","updated":"2022-12-27 23:18:40.000000000","message":"Done.","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000001,"name":"tazjin","email":"tazjin@tvl.su","username":"tazjin"},"change_message_id":"e1170e403b3eee3d9515d1d39a3f5dc1b70daf45","unresolved":true,"context_lines":[{"line_number":63,"context_line":"        // check directories"},{"line_number":64,"context_line":"        for directory_node in \u0026self.directories {"},{"line_number":65,"context_line":"            // check name for validity"},{"line_number":66,"context_line":"            if !Directory::name_is_valid(\u0026directory_node.name) {"},{"line_number":67,"context_line":"                return Err(ValidateDirectoryError::InvalidName("},{"line_number":68,"context_line":"                    directory_node.name.clone(),"},{"line_number":69,"context_line":"                ));"},{"line_number":70,"context_line":"            }"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"            // check digest to be 32 bytes"},{"line_number":73,"context_line":"            if directory_node.digest.len() !\u003d 32 {"},{"line_number":74,"context_line":"                return Err(ValidateDirectoryError::InvalidDigestLen("},{"line_number":75,"context_line":"                    directory_node.digest.len(),"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"d1fcbe85_8e3336f0","line":72,"range":{"start_line":66,"start_character":0,"end_line":72,"end_character":0},"updated":"2022-12-27 20:50:04.000000000","message":"I think rather than duplicating all of this logic so much you might want to have a helper function returning the `Result` already and call it as `helper(\u0026directory_node.name)?` (etc.)","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"8cb92694f8b0f8008a35c094ef14de8adaba7c98","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        // check directories"},{"line_number":64,"context_line":"        for directory_node in \u0026self.directories {"},{"line_number":65,"context_line":"            // check name for validity"},{"line_number":66,"context_line":"            if !Directory::name_is_valid(\u0026directory_node.name) {"},{"line_number":67,"context_line":"                return Err(ValidateDirectoryError::InvalidName("},{"line_number":68,"context_line":"                    directory_node.name.clone(),"},{"line_number":69,"context_line":"                ));"},{"line_number":70,"context_line":"            }"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"            // check digest to be 32 bytes"},{"line_number":73,"context_line":"            if directory_node.digest.len() !\u003d 32 {"},{"line_number":74,"context_line":"                return Err(ValidateDirectoryError::InvalidDigestLen("},{"line_number":75,"context_line":"                    directory_node.digest.len(),"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"aeffa7d3_a132f34c","line":72,"range":{"start_line":66,"start_character":0,"end_line":72,"end_character":0},"in_reply_to":"d1fcbe85_8e3336f0","updated":"2022-12-27 23:18:40.000000000","message":"I can move the name checks and error synthesis to the helper function, yes.","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000001,"name":"tazjin","email":"tazjin@tvl.su","username":"tazjin"},"change_message_id":"e1170e403b3eee3d9515d1d39a3f5dc1b70daf45","unresolved":true,"context_lines":[{"line_number":77,"context_line":"            }"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            // ensure names are sorted"},{"line_number":80,"context_line":"            if directory_node.name \u003c last_directory_name {"},{"line_number":81,"context_line":"                return Err(ValidateDirectoryError::WrongSorting("},{"line_number":82,"context_line":"                    directory_node.name.clone(),"},{"line_number":83,"context_line":"                ));"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"e85c2b0b_7d6bbf67","line":80,"range":{"start_line":80,"start_character":0,"end_line":80,"end_character":58},"updated":"2022-12-27 20:50:04.000000000","message":"same as above (and same below, won\u0027t comment on all fo them)","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"8cb92694f8b0f8008a35c094ef14de8adaba7c98","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            }"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            // ensure names are sorted"},{"line_number":80,"context_line":"            if directory_node.name \u003c last_directory_name {"},{"line_number":81,"context_line":"                return Err(ValidateDirectoryError::WrongSorting("},{"line_number":82,"context_line":"                    directory_node.name.clone(),"},{"line_number":83,"context_line":"                ));"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"f8eeddcf_711fd7f2","line":80,"range":{"start_line":80,"start_character":0,"end_line":80,"end_character":58},"in_reply_to":"e85c2b0b_7d6bbf67","updated":"2022-12-27 23:18:40.000000000","message":"Done","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000001,"name":"tazjin","email":"tazjin@tvl.su","username":"tazjin"},"change_message_id":"e1170e403b3eee3d9515d1d39a3f5dc1b70daf45","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            // ensure names are sorted"},{"line_number":80,"context_line":"            if directory_node.name \u003c last_directory_name {"},{"line_number":81,"context_line":"                return Err(ValidateDirectoryError::WrongSorting("},{"line_number":82,"context_line":"                    directory_node.name.clone(),"},{"line_number":83,"context_line":"                ));"},{"line_number":84,"context_line":"            }"},{"line_number":85,"context_line":"            last_directory_name \u003d directory_node.name.clone();"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"c5816091_3a326a95","line":82,"range":{"start_line":82,"start_character":40,"end_line":82,"end_character":48},"updated":"2022-12-27 20:50:04.000000000","message":"you don\u0027t have to `clone` here if you use references, which should work fine inside this entire function (the borrowed data outlives this function and nothing is returned from here)","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"},{"author":{"_account_id":1000036,"name":"flokli","email":"flokli@flokli.de","username":"flokli"},"change_message_id":"8cb92694f8b0f8008a35c094ef14de8adaba7c98","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            // ensure names are sorted"},{"line_number":80,"context_line":"            if directory_node.name \u003c last_directory_name {"},{"line_number":81,"context_line":"                return Err(ValidateDirectoryError::WrongSorting("},{"line_number":82,"context_line":"                    directory_node.name.clone(),"},{"line_number":83,"context_line":"                ));"},{"line_number":84,"context_line":"            }"},{"line_number":85,"context_line":"            last_directory_name \u003d directory_node.name.clone();"}],"source_content_type":"text/x-rustsrc","patch_set":3,"id":"17e47739_427cf198","line":82,"range":{"start_line":82,"start_character":40,"end_line":82,"end_character":48},"in_reply_to":"c5816091_3a326a95","updated":"2022-12-27 23:18:40.000000000","message":"Done.","commit_id":"b97453485208ca8adf59c599fedc5273da3a42ce"}]}