mark files generated by async_to_sync script as such in git

[dlax]

In order to make reviewing a bit more comfortable, see https://docs.github.com/en/repositories/working-with-files/managing-files/customizing-how-changed-files-appear-on-github

The async_to_sync.py script is extended to check that generated files are listed in .gitattributes.

[dvarrazzo]

Let's say I'm -0.5 about this.

I understand that this feature is used by github to filter out spurious languages of which the diff is not useful. Examples might be html files generated from source files and yet committed in the repos, vendored code (https://github.com/github-linguist/linguist/).

However our generated files is code that I think needs reviewing. Different, for instance, from CoffeeScript generating JS, or generated WASM, where the quality of the transformation is only of interest for the compier maintainers. async-to-sync does a few non-trivial operations such as changing the imported objects or managing the if ... # ASYNC branches, for instance in 0d1cd27:

--- a/psycopg_pool/psycopg_pool/pool_async.py
+++ b/psycopg_pool/psycopg_pool/pool_async.py
@@ -25,7 +25,7 @@ from .base import ConnectionAttempt, BasePool
 from .errors import PoolClosed, PoolTimeout, TooManyRequests
 from ._compat import Deque, Self
 from ._acompat import ACondition, AEvent, ALock, AQueue, AWorker, aspawn, agather
-from ._acompat import current_task_name
+from ._acompat import asleep, current_task_name
 from .sched_async import AsyncScheduler
 
 if True:  # ASYNC

--- a/psycopg_pool/psycopg_pool/pool.py
+++ b/psycopg_pool/psycopg_pool/pool.py
@@ -28,7 +28,7 @@ from .base import ConnectionAttempt, BasePool
 from .errors import PoolClosed, PoolTimeout, TooManyRequests
 from ._compat import Deque, Self
 from ._acompat import Condition, Event, Lock, Queue, Worker, spawn, gather
-from ._acompat import current_thread_name
+from ._acompat import sleep, current_thread_name
 from .sched import Scheduler

So here we need to review both async-to-sync job and that generated sync code as if we had written it ourselves.

Looking at a MR like #794, the autogenerated code is not a lot in the grand scheme of things: just two files over 11 in this case, and they are actually important, so I do have a preference of seeing them in the diffs.

So I think our repository is not the right audience for this linguist feature. Even from the PoV of why the linguist project seems to exist: it seems that it is made to compute stats about the language used in a repo, but here we generate python from python, which we would have it anyway even if it was handwritten; we don't get a dump of 10K lines of html because there's 1K lines of MD.

[asqui]

I agree with @dvarrazzo's thoughts -- I've actually had the exact same discussion regarding auto-generated code in an internal code base before, with the same conclusion that it is useful and important to be able to see and review in PRs the impact that changes to the generating code have on the generated code.

(For the sake of completeness of the argument, an even more extreme option would be to say that auto-generated code should not be checked-in to git at all, because it can just be generated on-demand from the code that is in git.)

[dlax]

It seems to me that we'll still be able to see and review files marked as generated; it's just that the github ui will hide their diff by default, but show a button to enable it; see for example facebookincubator/cinder@d2ea219#diff-1cd1f0bb26c3fbaa279f83ee2bfe1d9072da7af07407111c79039c66034c57ed or apache/datafusion@8730466#diff-12fbec27eb3beeb591bf4168774baeef425578819df241d20443bd973acd3f23 (random repos from a search on github).

The idea is definitely not to disallow review on generated files, rather making it easier when reviewing to identify such files and avoid commenting them when comments should actually go to the original version.

Yet I understand your points; and if you (still) both disagree on this change, I'm fine keeping thinks as is. Just let me know.