bug#81223: [PATCH] Fix file-missing error during async native compilation
James Cherti <[email protected]>
| Newsgroups | gmane.emacs.bugs |
|---|---|
| Message-ID | <[email protected]> |
Hello Eli and Sean, This patch is ready on my end, so feel free to merge it. My commit message is not that long, and the code already includes explanatory comments. I have also added the bug number in the latest patch version. What I personally dislike is that the standards Sean asked me to follow are not applied equally to everyone and seem based on personal preference. (For example, there are many long git commit messages and several commits that do not adhere to the CONTRIBUTE file.) Manually enforcing a two-space rule or commit message length limits is tedious and consumes valuable review time for Sean and other maintainers. If these specific conventions are required, they should be handled automatically by a linter or formatter rather than human reviewers. The way these small details are enforced right now is unfortunately inefficient and can cause friction. I recommend enforcing these minor formatting details through a pre-commit hook and a server-side check. This will ensure that the rules are applied consistently to all contributors, including maintainers. For contributors, this ensures that the pre-commit hook enforces these rules locally before a patch is submitted. This way, once a patch reaches the mailing list, reviewers can focus entirely on the code itself. On 2026-08-08 03:02, Eli Zaretskii wrote: >> Date: Fri, 7 Aug 2026 17:47:01 -0400 >> Cc: Eli Zaretskii <[email protected]>, Andrea Corallo <[email protected]>, >> [email protected], Sean Whitton <[email protected]> >> From: James Cherti <[email protected]> >> >> @Sean: I decline to make this change, as doing so would >> encourage counterproductive behavior within the community. > > Are you saying this because you feel there's some fundamental problem > with what Sean asked you to do, or because you are "fed up" with > requests for minor changes? If the former, please elaborate about > that problem. > >> The patch is ready to be merged. (Heavy nitpicking on small >> details, especially for simple patches, makes contributors >> hesitate before submitting future work.) > > See, that's not necessarily "nitpicking". One of our goals in making > such requests is to progressively make contributions adhere to our > standards as much as possible, with the purpose of eventually allowing > you and others write access to the tree, so you could install your > contributions with little or no peer review. Yes, this sometimes > results in seemingly-unnecessary nitpicking. For example, even the > latest variant of your patch leaves just one space between sentences, > whereas our conventions are to leave two spaces. > > It is true that we can make these changes when we install, but that > would be less optimal for several reasons: (a) you never learn these > small; but important details, and thus we cannot let you install your > patches any time soon; (b) some of your changes need to be followed by > correction commits because we cannot change the code you submit and > attribute the modified code to you; (c) if a problematic commit log > message is pushed before it is amended, the problems are forever > there, since Git doesn't have a way of editing commit log messages > after they are pushed. It also makes our job a tad harder, as we need > to carefully review these minor details and be alert to any minor > deviations, instead of just running "git am". > > So if you really feel you cannot cooperate with us in this way, it's > up to you, but I just wanted you to understand why we are asking for > these minor changes. > > Specifically, in this case, in addition to the two-spaces-between- > sentences issue (both in the commit log message and in the comments > that are part of your patch), there's this part of the commit log > message: > > The async native compilation queue can sometimes retain stale jobs for > files that no longer exist on disk (for example, when files are removed > during a package upgrade or deletion). Attempting to resolve the target > .eln filename for these missing files signals a file-missing error. > > This description belongs to the comments in the code, because it > explains why the code does a seemingly-strange thing. Part of this > explanation is already in your comments, which is good. But the part > which gives an example when this could happen is not in the comments, > and I think it should be added there. People who read the code should > be able to understand why it does what it does, without having to > resort to reading the relevant commits, I hope you agree with that. > > Are you okay with making these minor changes in your patch and posting > the result? -- James Cherti GitHub: https://github.com/jamescherti Website: https://www.jamescherti.com/