3ae7a3ee19
Patch Set 10:
(11 comments)
Patch-set: 10
Attention: {"person_ident":"Gerrit User 1088583 \u003c1088583@173816e5-2b9a-37c3-8a2e-48639d4f1153\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_1088583\u003e replied on the change"}
749 lines
No EOL
22 KiB
Text
749 lines
No EOL
22 KiB
Text
{
|
|
"comments": [
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "a7bd6762_f9f43bf7",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 206,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "To follow the same pattern used in other doc strings, such as L186.\n\n```\nfetch_start (float): The starting time.time() of the fetch operation.\n```",
|
|
"range": {
|
|
"startLine": 206,
|
|
"startChar": 19,
|
|
"endLine": 206,
|
|
"endChar": 69
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "68b72d45_337770da",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 206,
|
|
"author": {
|
|
"id": 4166729
|
|
},
|
|
"writtenOn": "2025-06-17T08:32:26Z",
|
|
"side": 1,
|
|
"message": "\u003e To follow the same pattern used in other doc strings, such as L186.\n\u003e \n\u003e ```\n\u003e fetch_start (float): The starting time.time() of the fetch operation.\n\u003e ```",
|
|
"parentUuid": "a7bd6762_f9f43bf7",
|
|
"range": {
|
|
"startLine": 206,
|
|
"startChar": 19,
|
|
"endLine": 206,
|
|
"endChar": 69
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "6e88b0fe_0e717c3a",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 206,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "68b72d45_337770da",
|
|
"range": {
|
|
"startLine": 206,
|
|
"startChar": 19,
|
|
"endLine": 206,
|
|
"endChar": 69
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "3c71e9bf_519ec48a",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 226,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "I\u0027d remove, as this is the result of the sync work that the worker performed.",
|
|
"range": {
|
|
"startLine": 226,
|
|
"startChar": 41,
|
|
"endLine": 226,
|
|
"endChar": 47
|
|
},
|
|
"fixSuggestions": [
|
|
{
|
|
"fixId": "43ed0d0f_0bbb20a7",
|
|
"description": "prompt_to_edit API",
|
|
"replacements": [
|
|
{
|
|
"path": "subcmds/sync.py",
|
|
"range": {
|
|
"startLine": 226,
|
|
"startChar": 0,
|
|
"endLine": 227,
|
|
"endChar": 0
|
|
},
|
|
"replacement": " \"\"\"The result of an interleaved sync.\n"
|
|
}
|
|
]
|
|
}
|
|
],
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "f2621492_be2015eb",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 226,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "By convention, Python doc strings tend to omit the leading \"the\" or \"a\" to make doc strings more compact and concise. I see that most of the doc-strings in this file adds a leading \"the\" and \"a\" in the arg section. However, for the class-level doc-strings, it seems that it tends to omit the leading \"the\"?\n\nSo...I\u0027d remove this `the`.",
|
|
"range": {
|
|
"startLine": 226,
|
|
"startChar": 4,
|
|
"endLine": 226,
|
|
"endChar": 11
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "5deac2c1_9a36a2ce",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 226,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "3c71e9bf_519ec48a",
|
|
"range": {
|
|
"startLine": 226,
|
|
"startChar": 41,
|
|
"endLine": 226,
|
|
"endChar": 47
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "8911e85b_6e18d408",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 226,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "f2621492_be2015eb",
|
|
"range": {
|
|
"startLine": 226,
|
|
"startChar": 4,
|
|
"endLine": 226,
|
|
"endChar": 11
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "472eb640_ff6e38b2",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2051,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "It looks like this function is to perform the sync, and doesn\u0027t model a worker. Also, the doc strings at L2053 sounds that this isn\u0027t a worker function, but sync function.\n\nCan this function be renamed to `_InterlevedSync()`, similar to `_SyncPhased()`?",
|
|
"range": {
|
|
"startLine": 2051,
|
|
"startChar": 11,
|
|
"endLine": 2051,
|
|
"endChar": 17
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "f0142119_c06077a9",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2051,
|
|
"author": {
|
|
"id": 4166729
|
|
},
|
|
"writtenOn": "2025-06-17T08:32:26Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "472eb640_ff6e38b2",
|
|
"range": {
|
|
"startLine": 2051,
|
|
"startChar": 11,
|
|
"endLine": 2051,
|
|
"endChar": 17
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "7edadae4_d078f63c",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2051,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "`_InterleavedSync` doesn\u0027t feel like the right naming since we have `_SyncInterleaved`. I renamed this to `_SyncProjectList` to match `_FetchProjectList`.",
|
|
"parentUuid": "f0142119_c06077a9",
|
|
"range": {
|
|
"startLine": 2051,
|
|
"startChar": 11,
|
|
"endLine": 2051,
|
|
"endChar": 17
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "1f265299_a1b6bd55",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2057,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "I\u0027d just copy the entire doc-string sentence for this arg. \n\n```\nopt: Program options returned from optparse. See _Options().\n```",
|
|
"range": {
|
|
"startLine": 2057,
|
|
"startChar": 12,
|
|
"endLine": 2057,
|
|
"endChar": 56
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "d9b4f98f_4c4edaa6",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2057,
|
|
"author": {
|
|
"id": 4166729
|
|
},
|
|
"writtenOn": "2025-06-17T08:32:26Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "1f265299_a1b6bd55",
|
|
"range": {
|
|
"startLine": 2057,
|
|
"startChar": 12,
|
|
"endLine": 2057,
|
|
"endChar": 56
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "aacd2c9d_ef9885d4",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2057,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "d9b4f98f_4c4edaa6",
|
|
"range": {
|
|
"startLine": 2057,
|
|
"startChar": 12,
|
|
"endLine": 2057,
|
|
"endChar": 56
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "a41eba5b_70a0a06a",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2070,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "maybe, it\u0027s good to add a check with `len(project_indices) \u003e 0` and return an empty result immediately. (or raise an exception if it must be a bug)\n\nOr, I\u0027d consider adding a check for `len(projects\") \u003d\u003d len(project_indices)` just to catch bugs in case.",
|
|
"range": {
|
|
"startLine": 2070,
|
|
"startChar": 33,
|
|
"endLine": 2070,
|
|
"endChar": 48
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "a8805c63_ffeba49d",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2070,
|
|
"author": {
|
|
"id": 4166729
|
|
},
|
|
"writtenOn": "2025-06-17T08:32:26Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "a41eba5b_70a0a06a",
|
|
"range": {
|
|
"startLine": 2070,
|
|
"startChar": 33,
|
|
"endLine": 2070,
|
|
"endChar": 48
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "1c688562_71928bab",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2070,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "Added an assert above. BTW we shouldn\u0027t be asserting `len(projects) \u003d\u003d len(project_indices)` since this worker is not guaranteed to process all the projects",
|
|
"parentUuid": "a8805c63_ffeba49d",
|
|
"range": {
|
|
"startLine": 2070,
|
|
"startChar": 33,
|
|
"endLine": 2070,
|
|
"endChar": 48
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "e2f4f504_eef9808f",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2094,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "This sync_dict is used to share the event start_time with the progress monitor thread. It already uses threading.Event to catch the events. I think it\u0027d be easier to debug and maintain the codes with multiplecess.Queue.\n\nFor instance, this worker would enqueue an item to notify the start event.\n```\ndef _InterleavedSyncWorker(..., progress_queue, ...):\n\n ...\n # notify the start to the monitor\n progress_queue.put({\n \"project\": first_project.name,\n \"relpath\": ...,\n \"start_time\": ..,\n })\n \n ...\n```",
|
|
"range": {
|
|
"startLine": 2094,
|
|
"startChar": 12,
|
|
"endLine": 2094,
|
|
"endChar": 30
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "b945fe5a_ff357ddd",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2094,
|
|
"author": {
|
|
"id": 4166729
|
|
},
|
|
"writtenOn": "2025-06-17T08:32:26Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "e2f4f504_eef9808f",
|
|
"range": {
|
|
"startLine": 2094,
|
|
"startChar": 12,
|
|
"endLine": 2094,
|
|
"endChar": 30
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "66b2968d_cff33a9a",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2094,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "I agree that using a Queue would improve long term maintainability, but it adds complexity to an already-working implementation. Added a TODO",
|
|
"parentUuid": "b945fe5a_ff357ddd",
|
|
"range": {
|
|
"startLine": 2094,
|
|
"startChar": 12,
|
|
"endLine": 2094,
|
|
"endChar": 30
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "af68054b_cbd3b574",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2173,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "I think it\u0027d be good to combine this logic and _GetSyncProgressMessasge() into a separate function. Then, _SyncInterleaved() can be shorter.\n\ne.g.,\n```\ndef _StartSyncProgressMonitor(wakeup_event: threading.Event) -\u003e _threading.Thread:\n pm \u003d Progress(\"Synching\", ...)\n \n def _progress_msg():\n ...\n \n def _loop():\n while True:\n pm.update(incr\u003d0, msg\u003d_progress_msg())\n if wakeup_event.wait(..):\n ...\n \n return _threading.Thread(target\u003d..., daemon\u003dTrue)\n```",
|
|
"range": {
|
|
"startLine": 2155,
|
|
"startChar": 7,
|
|
"endLine": 2173,
|
|
"endChar": 42
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "482d7f99_98b47895",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2173,
|
|
"author": {
|
|
"id": 4166729
|
|
},
|
|
"writtenOn": "2025-06-17T08:32:26Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "af68054b_cbd3b574",
|
|
"range": {
|
|
"startLine": 2155,
|
|
"startChar": 7,
|
|
"endLine": 2173,
|
|
"endChar": 42
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "baf0df71_c0b4868b",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2173,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "Deduped some logic into a new `_StartSyncProgressMonitor`.",
|
|
"parentUuid": "482d7f99_98b47895",
|
|
"range": {
|
|
"startLine": 2155,
|
|
"startChar": 7,
|
|
"endLine": 2173,
|
|
"endChar": 42
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "99404e77_c0300858",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2195,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "shoudln\u0027t this be an error log, as this is the error failed the sync?",
|
|
"range": {
|
|
"startLine": 2195,
|
|
"startChar": 31,
|
|
"endLine": 2195,
|
|
"endChar": 38
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "8fe97f7d_fa6f1ae1",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2195,
|
|
"author": {
|
|
"id": 4166729
|
|
},
|
|
"writtenOn": "2025-06-17T08:32:26Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "99404e77_c0300858",
|
|
"range": {
|
|
"startLine": 2195,
|
|
"startChar": 31,
|
|
"endLine": 2195,
|
|
"endChar": 38
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "45730479_74674214",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2195,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "8fe97f7d_fa6f1ae1",
|
|
"range": {
|
|
"startLine": 2195,
|
|
"startChar": 31,
|
|
"endLine": 2195,
|
|
"endChar": 38
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "317b65d2_738dc744",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2232,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "_ProcessSyncInterleavedResults have fields for errors. Can this logic iterate the results to catch the error, instead of using `err_event` to signal the main process that there was an error?",
|
|
"range": {
|
|
"startLine": 2232,
|
|
"startChar": 28,
|
|
"endLine": 2232,
|
|
"endChar": 37
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "f9f811d4_a948054a",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2232,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "`errors` are for data aggregation and are used in the final `SyncError` but `err_event` is for signaling/control flow, especially for --fail-fast. (this pattern is also used in _SyncPhased)",
|
|
"parentUuid": "317b65d2_738dc744",
|
|
"range": {
|
|
"startLine": 2232,
|
|
"startChar": 28,
|
|
"endLine": 2232,
|
|
"endChar": 37
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "6df57efe_66fa40da",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2244,
|
|
"author": {
|
|
"id": 1142791
|
|
},
|
|
"writtenOn": "2025-06-16T22:26:23Z",
|
|
"side": 1,
|
|
"message": "I think this is an optimization. Instead of looping the results to find the",
|
|
"range": {
|
|
"startLine": 2244,
|
|
"startChar": 28,
|
|
"endLine": 2244,
|
|
"endChar": 43
|
|
},
|
|
"fixSuggestions": [
|
|
{
|
|
"fixId": "57047c7e_b88eab30",
|
|
"description": "prompt_to_edit API",
|
|
"replacements": [
|
|
{
|
|
"path": "subcmds/sync.py",
|
|
"range": {
|
|
"startLine": 2244,
|
|
"startChar": 0,
|
|
"endLine": 2244,
|
|
"endChar": 0
|
|
},
|
|
"replacement": " # Set the error event immediately so that we don\u0027t\n # continue processing more projects. This avoids\n # unnecessary work if a failure has already occurred.\n"
|
|
}
|
|
]
|
|
}
|
|
],
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "5310ab21_a036045f",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2244,
|
|
"author": {
|
|
"id": 4166729
|
|
},
|
|
"writtenOn": "2025-06-17T08:32:26Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "6df57efe_66fa40da",
|
|
"range": {
|
|
"startLine": 2244,
|
|
"startChar": 28,
|
|
"endLine": 2244,
|
|
"endChar": 43
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "550c41a1_59a4e40e",
|
|
"filename": "subcmds/sync.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 2244,
|
|
"author": {
|
|
"id": 1088583
|
|
},
|
|
"writtenOn": "2025-06-17T17:25:52Z",
|
|
"side": 1,
|
|
"message": "I\u0027m assuming this is a leftover draft from the above comment: https://gerrit-review.git.corp.google.com/c/git-repo/+/483281/comment/317b65d2_738dc744/",
|
|
"parentUuid": "5310ab21_a036045f",
|
|
"range": {
|
|
"startLine": 2244,
|
|
"startChar": 28,
|
|
"endLine": 2244,
|
|
"endChar": 43
|
|
},
|
|
"revId": "fc39a70f4862454eeb13b92634782bf3a8230907",
|
|
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
|
|
}
|
|
]
|
|
} |