Tools/git-repo
1
0
Fork 0
Code Activity
git-repo/b46a3caae9ec8071a1e928261d0caf13797b741c
Gerrit User 1142791 c86146cd47 Update patch set 17 
Patch Set 17:

(1 comment)

Patch-set: 17
Attention: {"person_ident":"Gerrit User 1142791 \u003c1142791@173816e5-2b9a-37c3-8a2e-48639d4f1153\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_1142791\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 1088583 \u003c1088583@173816e5-2b9a-37c3-8a2e-48639d4f1153\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_1142791\u003e replied on the change"}
2025-06-17 22:10:44 -07:00

258 lines
No EOL
8.5 KiB
Text
Raw Blame History

{
"comments": [
{
"unresolved": true,
"key": {
"uuid": "667cb7e3_0b4e79c4",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2122,
"author": {
"id": 1142791
},
"writtenOn": "2025-06-17T19:13:24Z",
"side": 1,
"message": "Can this be just `fetch_error \u003d sync_result.error` without the if statement?",
"range": {
"startLine": 2121,
"startChar": 16,
"endLine": 2122,
"endChar": 51
},
"fixSuggestions": [
{
"fixId": "5c0ccf26_c04cd7f8",
"description": "prompt_to_edit API",
"replacements": [
{
"path": "subcmds/sync.py",
"range": {
"startLine": 2121,
"startChar": 0,
"endLine": 2123,
"endChar": 0
},
"replacement": " fetch_error \u003d sync_result.error\n"
}
]
}
],
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "ba153857_1433c39c",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2122,
"author": {
"id": 1088583
},
"writtenOn": "2025-06-17T22:05:23Z",
"side": 1,
"message": "Done",
"parentUuid": "667cb7e3_0b4e79c4",
"range": {
"startLine": 2121,
"startChar": 16,
"endLine": 2122,
"endChar": 51
},
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": true,
"key": {
"uuid": "f790fa60_a6383ce2",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2137,
"author": {
"id": 1142791
},
"writtenOn": "2025-06-17T19:13:24Z",
"side": 1,
"message": "I believe that the exception messages are informative enough and no custom log prefix is necessary. Also, it wouldn\u0027t be necessary to print the class name of the exception. How about below? \n\n```\ntry:\n ....\nexcept Exception as e:\n fetch_error \u003d e\n logger.error(\"fetch failed (%s): %s\", project.name, e)\nfinally:\n fetch_finish \u003d time.time()\n fetch_success \u003d fetch_error is None # Or, not(fetch_error) \n```\n\ne.g.,\n```\nfetch failed (\"android/cts\"): KeyboardInterrupt\nfetch failed (\"android/cts\"): not current on a branch\nfetch failed (\"android/cts\"): branch xxx does not track a remote\n```",
"range": {
"startLine": 2123,
"startChar": 12,
"endLine": 2137,
"endChar": 17
},
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": true,
"key": {
"uuid": "07484f1b_4e46da35",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2137,
"author": {
"id": 1088583
},
"writtenOn": "2025-06-17T22:05:23Z",
"side": 1,
"message": "a lot of this error handling is copied directly from\n\n_FetchOne: https://gerrit.googlesource.com/git-repo/+/f91f4462e6365b5545b39be597dab23619b8d291/subcmds/sync.py#774\n\nand _CheckoutOne: https://gerrit.googlesource.com/git-repo/+/f91f4462e6365b5545b39be597dab23619b8d291/subcmds/sync.py#1052\n\nbased on https://gerrit-review.googlesource.com/c/git-repo/+/303544 which added the `error.GitError: ` logging. WDYT about overhauling this for both sync modes in a followup CL?",
"parentUuid": "f790fa60_a6383ce2",
"range": {
"startLine": 2123,
"startChar": 12,
"endLine": 2137,
"endChar": 17
},
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "acb1639e_d8f05b73",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2137,
"author": {
"id": 1142791
},
"writtenOn": "2025-06-18T01:43:58Z",
"side": 1,
"message": "sgtm",
"parentUuid": "07484f1b_4e46da35",
"range": {
"startLine": 2123,
"startChar": 12,
"endLine": 2137,
"endChar": 17
},
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": true,
"key": {
"uuid": "b9796afa_c36d8115",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2188,
"author": {
"id": 1142791
},
"writtenOn": "2025-06-17T19:13:24Z",
"side": 1,
"message": "same. one exception block would be enough",
"range": {
"startLine": 2171,
"startChar": 10,
"endLine": 2188,
"endChar": 20
},
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "83908b98_b344633e",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2188,
"author": {
"id": 1088583
},
"writtenOn": "2025-06-17T22:05:23Z",
"side": 1,
"message": "see thread above for discussion",
"parentUuid": "b9796afa_c36d8115",
"range": {
"startLine": 2171,
"startChar": 10,
"endLine": 2188,
"endChar": 20
},
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "9120fbe8_a4f67c1b",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2358,
"author": {
"id": 1142791
},
"writtenOn": "2025-06-17T19:13:24Z",
"side": 1,
"message": "I feel that self.ParallelContext() should yield the context, so that this can be as follows.\n\n```\nwith self.ParallelContext() as context:\n context[\"ssh_proxy\"] ssh_proxy\n```",
"range": {
"startLine": 2357,
"startChar": 12,
"endLine": 2358,
"endChar": 44
},
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "e5ffe058_23f5d7b4",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2358,
"author": {
"id": 1088583
},
"writtenOn": "2025-06-18T02:34:49Z",
"side": 1,
"message": "Missed this from last time, but:\n```\n with self.ParallelContext() as context:\n\u003e context[\"ssh_proxy\"] \u003d ssh_proxy\n ^^^^^^^^^^^^^^^^^^^^\nE TypeError: \u0027NoneType\u0027 object does not support item assignment\n```",
"parentUuid": "9120fbe8_a4f67c1b",
"range": {
"startLine": 2357,
"startChar": 12,
"endLine": 2358,
"endChar": 44
},
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "3d5fd7e0_127e75fe",
"filename": "subcmds/sync.py",
"patchSetId": 15
},
"lineNbr": 2358,
"author": {
"id": 1142791
},
"writtenOn": "2025-06-18T05:10:44Z",
"side": 1,
"message": "that\u0027s what I meant. The following line could yield the context.\n- https://gerrit.googlesource.com/git-repo/+/refs/tags/v2.55.2/command.py#266\n\n```\ndef ParallelContext(cls):\n cls._parallel_context \u003d {}\n try:\n yield cls._parallel_context\n finally:\n cls._parallel_context \u003d None\n```\n```suggestion\n with self.ParallelContext():\n self.get_parallel_context()[\"ssh_proxy\"] \u003d ssh_proxy\n```\n\njust to be clear, I didn\u0027t mean to suggest making the change in this CL.",
"parentUuid": "e5ffe058_23f5d7b4",
"range": {
"startLine": 2357,
"startChar": 12,
"endLine": 2358,
"endChar": 44
},
"revId": "b46a3caae9ec8071a1e928261d0caf13797b741c",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
}
]
}
View git blame Copy permalink