1
0
Fork 0
git-repo/14bf8434de714f0381eabc93a4d2735d5ff1331b
Gerrit User 4127492 dbc9a6cb5f Update patch set 13
Patch Set 13:

(6 comments)

Patch-set: 13
Attention: {"person_ident":"Gerrit User 4127492 \u003c4127492@173816e5-2b9a-37c3-8a2e-48639d4f1153\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_4127492\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 1007145 \u003c1007145@173816e5-2b9a-37c3-8a2e-48639d4f1153\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_4127492\u003e replied on the change"}
2025-06-05 16:28:47 -07:00

272 lines
No EOL
8.6 KiB
Text
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

{
"comments": [
{
"unresolved": true,
"key": {
"uuid": "59c9c9c4_89d60e08",
"filename": "/COMMIT_MSG",
"patchSetId": 12
},
"lineNbr": 29,
"author": {
"id": 1007145
},
"writtenOn": "2025-06-05T19:44:07Z",
"side": 1,
"message": "missing `Bug: 421694721` tag",
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "d860e6c2_56cd5edf",
"filename": "/COMMIT_MSG",
"patchSetId": 12
},
"lineNbr": 29,
"author": {
"id": 4127492
},
"writtenOn": "2025-06-05T23:28:47Z",
"side": 1,
"message": "Done",
"parentUuid": "59c9c9c4_89d60e08",
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": true,
"key": {
"uuid": "07cb7682_677369b6",
"filename": "docs/repo-hooks.md",
"patchSetId": 12
},
"lineNbr": 140,
"author": {
"id": 1007145
},
"writtenOn": "2025-06-05T19:44:07Z",
"side": 1,
"message": "this text is leaving a ton unspecified\n\nthe hook, as written, runs whenever `repo sync ...` finishes without errors. that does not mean every project was downloaded \u0026 checked out.\n\nif someone runs `repo sync -n` to only download from the network, then nothing in the checkout will be touched. so it might be completely empty.\n\nif someone runs `repo sync foo` to only download \u0026 checkout the `foo` project, then nothing else in the checkout will be touched.\n\nhttp://gerrit-review.googlesource.com/c/277563/ gets into this nuance too.",
"range": {
"startLine": 139,
"startChar": 0,
"endLine": 140,
"endChar": 24
},
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "2125c86f_a658b237",
"filename": "docs/repo-hooks.md",
"patchSetId": 12
},
"lineNbr": 140,
"author": {
"id": 4127492
},
"writtenOn": "2025-06-05T23:28:47Z",
"side": 1,
"message": "Thanks again — I updated the doc to clarify that `post-sync` runs after a successful `repo sync`, but doesnt imply all projects were synced or checked out.\n\nIncluded examples like `repo sync -n` and single-project sync to make that nuance clear.\n\nLet me know if the new wording looks better!",
"parentUuid": "07cb7682_677369b6",
"range": {
"startLine": 139,
"startChar": 0,
"endLine": 140,
"endChar": 24
},
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": true,
"key": {
"uuid": "85743790_92615476",
"filename": "docs/repo-hooks.md",
"patchSetId": 12
},
"lineNbr": 160,
"author": {
"id": 1007145
},
"writtenOn": "2025-06-05T19:44:07Z",
"side": 1,
"message": "please copy the docstring over from pre-upload",
"fixSuggestions": [
{
"fixId": "16d51b36_ffc2cb28",
"description": "prompt_to_edit API",
"replacements": [
{
"path": "docs/repo-hooks.md",
"range": {
"startLine": 161,
"startChar": 0,
"endLine": 161,
"endChar": 0
},
"replacement": " \"\"\"Main function invoked directly by repo.\n\n We must use the name \"main\" as that is what repo requires.\n\n Args:\n kwargs: Leave this here for forward-compatibility.\n \"\"\"\n"
}
]
}
],
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "502e4435_bea92ca6",
"filename": "docs/repo-hooks.md",
"patchSetId": 12
},
"lineNbr": 160,
"author": {
"id": 4127492
},
"writtenOn": "2025-06-05T23:28:47Z",
"side": 1,
"message": "Fix applied.",
"parentUuid": "85743790_92615476",
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": true,
"key": {
"uuid": "be0623aa_860ab365",
"filename": "subcmds/sync.py",
"patchSetId": 12
},
"lineNbr": 1741,
"author": {
"id": 1007145
},
"writtenOn": "2025-06-05T19:44:07Z",
"side": 1,
"message": "this shouldn\u0027t be in the try block",
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "78f3ab22_7858f073",
"filename": "subcmds/sync.py",
"patchSetId": 12
},
"lineNbr": 1741,
"author": {
"id": 4127492
},
"writtenOn": "2025-06-05T23:28:47Z",
"side": 1,
"message": "- Removed outer try/except around `_RunPostSyncHook()`\n- Relying on `RepoHook.Run()` return value to detect failure, as suggested\n- Confirmed `post-sync` only runs after successful sync, and doesn\u0027t affect error flow\n\nThanks again for the detailed feedback — let me know if anything else needs tweaking!",
"parentUuid": "be0623aa_860ab365",
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": true,
"key": {
"uuid": "e25d6a19_bf3fb817",
"filename": "subcmds/sync.py",
"patchSetId": 12
},
"lineNbr": 1751,
"author": {
"id": 1007145
},
"writtenOn": "2025-06-05T19:44:07Z",
"side": 1,
"message": "just call it `hook`",
"fixSuggestions": [
{
"fixId": "29ae4efd_6be4016d",
"description": "prompt_to_edit API",
"replacements": [
{
"path": "subcmds/sync.py",
"range": {
"startLine": 1751,
"startChar": 0,
"endLine": 1752,
"endChar": 0
},
"replacement": " hook \u003d RepoHook.FromSubcmd(\n"
},
{
"path": "subcmds/sync.py",
"range": {
"startLine": 1757,
"startChar": 0,
"endLine": 1758,
"endChar": 0
},
"replacement": " hook.Run()\n"
}
]
}
],
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "3e676db7_07d3153b",
"filename": "subcmds/sync.py",
"patchSetId": 12
},
"lineNbr": 1751,
"author": {
"id": 4127492
},
"writtenOn": "2025-06-05T23:28:47Z",
"side": 1,
"message": "Done",
"parentUuid": "e25d6a19_bf3fb817",
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": true,
"key": {
"uuid": "eecc2f74_eec94755",
"filename": "subcmds/sync.py",
"patchSetId": 12
},
"lineNbr": 1759,
"author": {
"id": 1007145
},
"writtenOn": "2025-06-05T19:44:07Z",
"side": 1,
"message": "this try/except block is useless as the Run method already catches everything it should. the only thing the caller should look at is the bool return value.",
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
},
{
"unresolved": false,
"key": {
"uuid": "18d49a7b_61fd0b3c",
"filename": "subcmds/sync.py",
"patchSetId": 12
},
"lineNbr": 1759,
"author": {
"id": 4127492
},
"writtenOn": "2025-06-05T23:28:47Z",
"side": 1,
"message": "Thanks — good point.\n\nI removed the outer try/except from `_RunPostSyncHook()`, and now rely on the boolean return value from `Run()` to detect failure, as `RepoHook` already handles exceptions internally.",
"parentUuid": "eecc2f74_eec94755",
"revId": "14bf8434de714f0381eabc93a4d2735d5ff1331b",
"serverId": "173816e5-2b9a-37c3-8a2e-48639d4f1153"
}
]
}