5 Commits

Author SHA1 Message Date
Gerrit User 1450817
36b9f9216d Update patch set 8
Patch Set 8:

(1 comment)

Patch-set: 8
2026-01-07 17:13:01 -08:00
Gerrit User 1450817
5b8d8e72a9 Create patch set 8
Uploaded patch set 8.

Patch-set: 8
Subject: Implement a universal formatter for depot_tools.
Commit: 1304623d0e
Tag: autogenerated:gerrit:newPatchSet
Groups: 3256e818da
Attention: {"person_ident":"Gerrit User 1303432 \u003c1303432@3ce6091f-6c88-37e8-8c75-72f92ae8dfba\u003e","operation":"ADD","reason":"Someone else replied on a comment you posted"}
2026-01-07 17:13:01 -08:00
Gerrit User 1450817
24edf34f34 Update patch set 7
Patch Set 7:

(14 comments)

Patch-set: 7
2026-01-07 15:32:50 -08:00
Gerrit User 1450817
151e865b42 Create patch set 7
Uploaded patch set 7.

Patch-set: 7
Subject: Implement a universal formatter for depot_tools.
Commit: b87046e711
Tag: autogenerated:gerrit:newPatchSet
Groups: 3256e818da
Attention: {"person_ident":"Gerrit User 1450817 \u003c1450817@3ce6091f-6c88-37e8-8c75-72f92ae8dfba\u003e","operation":"REMOVE","reason":"removed on reply"}
Attention: {"person_ident":"Gerrit User 1000768 \u003c1000768@3ce6091f-6c88-37e8-8c75-72f92ae8dfba\u003e","operation":"ADD","reason":"Someone else replied on a comment you posted"}
2026-01-07 15:32:50 -08:00
Gerrit User 1303432
f038d1f644 Update patch set 6
Patch Set 6:

(1 comment)

Patch-set: 6
Reviewer: Gerrit User 3270325 <3270325@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Attention: {"person_ident":"Gerrit User 3270325 \u003c3270325@3ce6091f-6c88-37e8-8c75-72f92ae8dfba\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_1303432\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 1303432 \u003c1303432@3ce6091f-6c88-37e8-8c75-72f92ae8dfba\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_1303432\u003e replied on the change"}
2026-01-07 15:25:11 -08:00

View File

@@ -1,5 +1,40 @@
{
"comments": [
{
"unresolved": false,
"key": {
"uuid": "ff987cad_f4e7bc5e",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 6
},
"lineNbr": 0,
"author": {
"id": 1303432
},
"writtenOn": "2026-01-07T23:25:11Z",
"side": 1,
"message": "I believe we\u0027ve already made the git cl format code fairly generic, in that it\u0027s called from Cider G which is not git with various diffs, etc. Do we need to add a new tool at all? What\u0027s preventing you from just using (or refactoring) the existing depot_tools formatting code used by git cl.",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
"uuid": "78831e34_418bbb0d",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 6
},
"lineNbr": 0,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-08T01:13:01Z",
"side": 1,
"message": "I did refactor the common parts of `git_cl`, but there were some issues with just using the `Run*Format` functions directly:\n* They call `DieWithError` if they can\u0027t find the formatter. We want to fallback to skipping formatting\n* Those functions pass the filename so that they read from said file. JJ requires the file to be read from stdin (since jj is designed to format files in commits that aren\u0027t on disk).\n* Those functions write the output back to the file. JJ requires them to write it to stdout (since again, JJ may format files that aren\u0027t on disk).\n\nThe `DieWithError` could be worked around, but it\u0027s a rather fundamental change to change it to use stdin / stdout instead of passing a file path.\n\nAlso, it turns out that almost all of the complexity of the formatters (with the exception of the python formatter) comes from `git cl format` trying to handle options, which `jj fix` has no intention of supporting. Our command to run the formatters usually just boils down to `run_formatter([FindTool()])`",
"parentUuid": "ff987cad_f4e7bc5e",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@@ -71,6 +106,54 @@
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "8c17b184_6c11ee19",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 15,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Does go/google3format suffice? My design was basically:\n* Copy the user interface of google3format, because we have the same considerations as them\n* Copy the feature set of `git cl format`, since this is designed to be an alternative to `git cl format` (a way of formatting files for people not using git).\n\nIf you\u0027d like, I can move this to a jj directory, since although the spec isn\u0027t specific to jj I assume jj will be the main user. `jj fix` always passes the file to stdin and provides the filename. Users should never run `./format foo.py`. Instead they (if using jj) will run `jj fix foo.py` (or `jj fix` to format all modified files).",
"parentUuid": "e6a68ec1_863b91b4",
"range": {
"startLine": 13,
"startChar": 0,
"endLine": 15,
"endChar": 54
},
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
"uuid": "e279dd2f_1ce11a03",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 15,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "I based this upon go/google3format, which notably does not support `google3format foo.py`. It requires that you instead run `cat foo.cc | google3format --depot_path //depot/path/foo.cc --whole-file \u003e foo.cc`.\n\ngo/google3format specifically says in the requirements for the tool that it must:\n* Take only the exact original file content on stdin.\n* Give only the exact formatted file content on stdout.\n\nIt does not support passing a list of files directly, as:\n* It is not designed to be used by an end-user directly\n* It is designed to be able to be ran in parallel by other tools\n* It does not want to require files to be materialized on disk\n\nThis formatter follows the opinion of google3format that a formatter is a tool that the user cannot choose to skip, and thus the user should not run it manually but instead it should be run for the user. Specifically, `jj upload` runs formatters as a pre-upload hook.\n\nBoth `hg fix` and `jj fix` run formatters on commits that may not be materialrized on disk. If I have a stack of commits that I\u0027m about to upload, for example, I want to format each commit in the stack rather than just format what\u0027s on disk (which is precisely what `jj fix` does).",
"parentUuid": "e6a68ec1_863b91b4",
"range": {
"startLine": 13,
"startChar": 0,
"endLine": 15,
"endChar": 54
},
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@@ -88,6 +171,24 @@
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "7de82bcc_613679dd",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 41,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "fc874cbf_52a817f6",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@@ -141,6 +242,42 @@
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "94d23a6d_5de258c5",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 46,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "3cd923d0_354d53fe",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "54370d6a_be2c3fca",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 46,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "3cd923d0_354d53fe",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@@ -158,6 +295,42 @@
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "13e25e2f_8303f4a5",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 51,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "6309e84d_4c0155b5",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "79836352_f31b6c98",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 51,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "6309e84d_4c0155b5",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@@ -211,6 +384,24 @@
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "1151e289_e32cb4f4",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 86,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "I checked and on windows you do need the interpreter. `subprocess.run` complains that it\u0027s not a valid win32 application, and with `shell\u003dTrue` it opens up the \"select an app to open this .py file\" prompt.",
"parentUuid": "4f6dc181_b7374244",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@@ -264,6 +455,24 @@
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "f230d583_3965ba1c",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 127,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "I\u0027ve added a comment that this is something that we could do in the future, but that we didn\u0027t want to do it now to match `git cl format` behavior.",
"parentUuid": "297174c3_b684c4bc",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@@ -281,6 +490,42 @@
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "44e247a9_ed90830a",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 139,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "22a8f7ca_c04eec8a",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "dfd218d5_5c3d9fb0",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 139,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "22a8f7ca_c04eec8a",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@@ -298,6 +543,24 @@
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "cf05b514_25f9103d",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 140,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "6d68ef54_cd0b8086",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@@ -314,6 +577,42 @@
"message": "why is this talking about jj ? nothing in here is specific to jj, nor is that tool required to do development anywhere.",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "4a486f18_b786b337",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 142,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "a105714e_18372637",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "0d8cfb6c_c861b766",
"filename": "format.py",
"patchSetId": 6
},
"lineNbr": 142,
"author": {
"id": 1450817
},
"writtenOn": "2026-01-07T23:32:50Z",
"side": 1,
"message": "Done",
"parentUuid": "a105714e_18372637",
"revId": "622ee23b86b073429f903968312c3f92757913ea",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
}
]
}