)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"e52d5badf13c5c54af55a10638a6f84ffe0fe649","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Replace our hand-rolled conversion utilities with the built-ins from"},{"line_number":10,"context_line":"configparser. Note that this slightly changes our behavior as `on` and"},{"line_number":11,"context_line":"`yes` are now valid boolean values, but that seems reasonable."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I1bb0457df23ef9772ab4548de7dcc8f7d3e2e8bd"},{"line_number":14,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"b1c27f7d_67da8aa2","line":11,"updated":"2026-10-01 15:16:28.000000000","message":"Agreed. This is an additive behavior change which shouldn\u0027t affect existing user configs. Seems safe enough.","commit_id":"08a6451f0380892a552491b86fa625b85d5807da"}],"git_review/cmd.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"e52d5badf13c5c54af55a10638a6f84ffe0fe649","unresolved":true,"context_lines":[{"line_number":349,"context_line":"        value \u003d git_config_get_value(\u0027gitreview\u0027, key)"},{"line_number":350,"context_line":"        if value is None:"},{"line_number":351,"context_line":"            value \u003d self.config[key]"},{"line_number":352,"context_line":"        elif key in _CONFIG_TYPES:"},{"line_number":353,"context_line":"            type_ \u003d _CONFIG_TYPES[key]"},{"line_number":354,"context_line":"            if type_ is bool:"},{"line_number":355,"context_line":"                value \u003d value.lower() in (\u0027true\u0027, \u00271\u0027, \u0027yes\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"47b3aeb1_0732f133","line":352,"range":{"start_line":352,"start_character":8,"end_line":352,"end_character":12},"updated":"2026-10-01 15:16:28.000000000","message":"Is the elif here correct or do we want to apply the same consistent conversion rules regardless of whether the config is in the .git/config or in the self.config file (which I think is .gitreview)? If we want the apply the rules consistently then I think we want this to be an if not an elif to apply to values loaded from both locations.\n\nOr maybe we have already applied the schema to self.config via load_config_file so this isn\u0027t necessary?","commit_id":"08a6451f0380892a552491b86fa625b85d5807da"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"33fd2f3a1f731949288cc85ca88d3fef7707aaab","unresolved":true,"context_lines":[{"line_number":349,"context_line":"        value \u003d git_config_get_value(\u0027gitreview\u0027, key)"},{"line_number":350,"context_line":"        if value is None:"},{"line_number":351,"context_line":"            value \u003d self.config[key]"},{"line_number":352,"context_line":"        elif key in _CONFIG_TYPES:"},{"line_number":353,"context_line":"            type_ \u003d _CONFIG_TYPES[key]"},{"line_number":354,"context_line":"            if type_ is bool:"},{"line_number":355,"context_line":"                value \u003d value.lower() in (\u0027true\u0027, \u00271\u0027, \u0027yes\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"d428f6d2_55695918","line":352,"range":{"start_line":352,"start_character":8,"end_line":352,"end_character":12},"in_reply_to":"47b3aeb1_0732f133","updated":"2026-10-02 09:34:15.000000000","message":"No, this is correct. This function will only be called if the attribute isn\u0027t explicitly set on the object. We explicitly set attributes below in `load_config_file` and we\u0027ve already updated that so that we will always store the expected type (`str`, `bool` or `int`). Changing this to `if` would cause us to attempt to call e.g. `.lower()` on a `bool`.\n\nMaybe it would have been clearer to write this as:\n\n```suggestion\n        else:\n            if key in _CONFIG_TYPES:\n```\n\nor to invert the nullness check?\n\n```\nif value is not None:\n    if key in _CONFIG_TYPES:\n        ...\nelse:\n    value \u003d self.config[key]\n```\n\n\nCan address this in a follow-up if so.","commit_id":"08a6451f0380892a552491b86fa625b85d5807da"}]}