)]}'
{"git_review/cmd.py":[{"author":{"_account_id":2463,"name":"Florian Haas","email":"florian.haas@cleura.com","username":"fghaas"},"change_message_id":"f2ac75eb44cfff7bde02449e55916afab1aed4f4","unresolved":true,"context_lines":[{"line_number":387,"context_line":"        run_command_exc("},{"line_number":388,"context_line":"            CannotInstallHook,"},{"line_number":389,"context_line":"            \"git\", \"submodule\", \"foreach\","},{"line_number":390,"context_line":"            \u0027cp -p %s \"$(git rev-parse --git-dir)/hooks/\"\u0027 % target_file)"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    if not os.access(target_file, os.X_OK):"},{"line_number":393,"context_line":"        os.chmod(target_file, os.path.stat.S_IREAD | os.path.stat.S_IEXEC)"}],"source_content_type":"text/x-python","patch_set":1,"id":"0feb77a9_32a11b73","line":390,"updated":"2021-06-16 19:10:03.000000000","message":"I *think* we can let this one slide. I don\u0027t think anyone runs \"git config core.hooksPath\" in a submodule checkout. Happy to add a comment here though, or add a note to the commit message.","commit_id":"7ea70c1ffb4fc26e5be401b56ce788cd4de8b01d"},{"author":{"_account_id":2463,"name":"Florian Haas","email":"florian.haas@cleura.com","username":"fghaas"},"change_message_id":"f2e080207bfa8dd0273b7fcca0e0a1ed7d36bbab","unresolved":true,"context_lines":[{"line_number":387,"context_line":"        run_command_exc("},{"line_number":388,"context_line":"            CannotInstallHook,"},{"line_number":389,"context_line":"            \"git\", \"submodule\", \"foreach\","},{"line_number":390,"context_line":"            \u0027cp -p %s \"$(git rev-parse --git-dir)/hooks/\"\u0027 % target_file)"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    if not os.access(target_file, os.X_OK):"},{"line_number":393,"context_line":"        os.chmod(target_file, os.path.stat.S_IREAD | os.path.stat.S_IEXEC)"}],"source_content_type":"text/x-python","patch_set":1,"id":"741a4317_9f1805d7","line":390,"in_reply_to":"0feb77a9_32a11b73","updated":"2021-06-16 19:19:54.000000000","message":"Went with an additional comment in patchset 2.","commit_id":"7ea70c1ffb4fc26e5be401b56ce788cd4de8b01d"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"1609d1b16bf7bba641dc2ae8868a1bbb76022e41","unresolved":false,"context_lines":[{"line_number":386,"context_line":"        # each of them."},{"line_number":387,"context_line":"        # Here, we don\u0027t check for any nonstandard hooks path, because"},{"line_number":388,"context_line":"        # it should be safe to assume that very few users are inclined"},{"line_number":389,"context_line":"        # to set the core.hooksPath option in a submodule checkout."},{"line_number":390,"context_line":"        run_command_exc("},{"line_number":391,"context_line":"            CannotInstallHook,"},{"line_number":392,"context_line":"            \"git\", \"submodule\", \"foreach\","}],"source_content_type":"text/x-python","patch_set":3,"id":"2a48ed87_71d42c33","line":389,"updated":"2021-06-18 23:44:20.000000000","message":"Also, perhaps obviously, if it\u0027s set globally in their ~/.gitconfig then they will already have installed the hook there by the time we reach this point.","commit_id":"9b57c80ade64e6953c844ab739cbef4473c8aad6"}],"git_review/tests/test_git_review.py":[{"author":{"_account_id":2463,"name":"Florian Haas","email":"florian.haas@cleura.com","username":"fghaas"},"change_message_id":"37a018abeae2fb81e5d6043c9e4fdc490a29e3e3","unresolved":true,"context_lines":[{"line_number":109,"context_line":"        \"\"\""},{"line_number":110,"context_line":"        hooks_subdir \u003d \".git/hooks\""},{"line_number":111,"context_line":"        self.reset_remote()"},{"line_number":112,"context_line":"        # What we ought to be doing here, to be sure we don\u0027t have a"},{"line_number":113,"context_line":"        # core.hooksPath option set, is this:"},{"line_number":114,"context_line":"        # self._run_git(\"config\", \"--unset\", \"core.hooksPath\")"},{"line_number":115,"context_line":"        # But that appears to break the test, so we leave it and"}],"source_content_type":"text/x-python","patch_set":7,"id":"009a10e4_63fd7da3","line":112,"updated":"2021-06-21 08:24:11.000000000","message":"If any reviewer has an idea on how to make this work *with* the explicit \"git config --unset\", I\u0027d be grateful for those. \n\nIf conversely we can guarantee that every time the test runs it will have a fresh config, then I can also remove this comment as it would be addressing a non-issue.","commit_id":"94f4ae33abf0b3c7867006b3ca35225fd8294768"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"c7e1389961c55929f01c8bb5c0d78e0a538509af","unresolved":true,"context_lines":[{"line_number":109,"context_line":"        \"\"\""},{"line_number":110,"context_line":"        hooks_subdir \u003d \".git/hooks\""},{"line_number":111,"context_line":"        self.reset_remote()"},{"line_number":112,"context_line":"        # What we ought to be doing here, to be sure we don\u0027t have a"},{"line_number":113,"context_line":"        # core.hooksPath option set, is this:"},{"line_number":114,"context_line":"        # self._run_git(\"config\", \"--unset\", \"core.hooksPath\")"},{"line_number":115,"context_line":"        # But that appears to break the test, so we leave it and"}],"source_content_type":"text/x-python","patch_set":7,"id":"b3019222_d9bbaa5c","line":112,"in_reply_to":"009a10e4_63fd7da3","updated":"2021-06-21 13:38:45.000000000","message":"If I interpret my local test correctly, the issue is that the \"git config --unset\" returns an error code when the variable wasn\u0027t set before: \n\nuser@xps1:~/src/git-review$ git config --unset core.hooksPath\nuser@xps1:~/src/git-review$ echo $?\n5\n\nSo the solution would be to patch utils.run_cmd() to accept that return code as not being an error. Not sure whether it is worth the effort, though. Maybe just add the unset at the end of the tests where we set that variable instead?","commit_id":"94f4ae33abf0b3c7867006b3ca35225fd8294768"},{"author":{"_account_id":2463,"name":"Florian Haas","email":"florian.haas@cleura.com","username":"fghaas"},"change_message_id":"b01d713f27905a5885cc1eff9c0f802a4641f6a9","unresolved":true,"context_lines":[{"line_number":109,"context_line":"        \"\"\""},{"line_number":110,"context_line":"        hooks_subdir \u003d \".git/hooks\""},{"line_number":111,"context_line":"        self.reset_remote()"},{"line_number":112,"context_line":"        # What we ought to be doing here, to be sure we don\u0027t have a"},{"line_number":113,"context_line":"        # core.hooksPath option set, is this:"},{"line_number":114,"context_line":"        # self._run_git(\"config\", \"--unset\", \"core.hooksPath\")"},{"line_number":115,"context_line":"        # But that appears to break the test, so we leave it and"}],"source_content_type":"text/x-python","patch_set":7,"id":"9b22928b_625af878","line":112,"in_reply_to":"b3019222_d9bbaa5c","updated":"2021-06-21 13:44:27.000000000","message":"\u003e user@xps1:~/src/git-review$ git config --unset core.hooksPath\n\u003e user@xps1:~/src/git-review$ echo $?\n\u003e 5\n\nConfirmed. Head→desk. Gotta love nonzero exit codes without so much as a hint on stderr. 🤷‍♂️","commit_id":"94f4ae33abf0b3c7867006b3ca35225fd8294768"}]}