On Fri, May 23, 2025, at 00:02, Mark Mentovai wrote:
> Commit 01aff0a (apply: correctly reverse patch's pre- and post-image
> mode bits; 2023-12-26) revised reverse_patches() to maintain the desired
The way the commit is referred to is almost like the usual
and recommended
git show -s --pretty=reference
But with a semicolon instead of a comma.
> property that when only one of patch::old_mode and patch::new_mode is
> set, the mode will be carried in old_mode. That property is generally
> correct, with one notable notable exception: when creating a file, only
s/notable notable/notable/
> new_mode will be set. Since reversing a deletion results in a creation,
> new_mode must be set in that case.
>
> Omitting handling for this case meant that reversing a patch that
> removed an executable file would not result in the executable permission
> being set on the re-created file.
>
> When git apply --reverse is used, reverse_patches() will now additionaly
> swap old_mode and new_mode for what's represented in the patch as a file
> deletion, as it is transformed into a file creation under reversal.
The usual way to refer to code behavior is to talk about the code
without this patch/commit in the present tense. I think this is talking
about how the code behaves with this patch applied/with this commit.
In my opinion it helps the narrative flow since something right-now is
problematic. Therefore (see next point) do this and that to fix the
situation.
> Tests are added that ensure that git apply sets file modes correctly on
> file creation, both in the normal (forward) and reverse direction.
> Existing test coverage for file modes focused only on mode changes of
> existing files, and only in the forward direction.
It’s recommended to describe changes as “do this and that” to the code.
“Add tests”, not “added tests” or “tests are added” (and the latter here
seems to use a passive construct that doesn’t feel in line with the
preceding paragraphs).
--
Kristoffer
