| Message ID | ad9ee00a2a971522968f95dd413deae24839ef71.1729017728.git.code@khaugsbakk.name (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | doc: update-ref: amend old material and discuss symrefs | expand |
On Tue, Oct 15, 2024 at 09:03:10PM +0200, kristofferhaugsbakk@fastmail.com wrote: > From: Kristoffer Haugsbakk <code@khaugsbakk.name> > > The other paragraphs on options say `With <option>,`. Let’s be uniform. > > Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> > --- > Documentation/git-update-ref.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt > index afcf33cf608..fe5967234e9 100644 > --- a/Documentation/git-update-ref.txt > +++ b/Documentation/git-update-ref.txt > @@ -55,7 +55,7 @@ for reading but not for writing (so we'll never write through a > ref symlink to some other tree, if you have copied a whole > archive by creating a symlink tree). > > -With `-d` flag, it deletes the named <ref> after verifying it > +With `-d`, it deletes the named <ref> after verifying it > still contains <old-oid>. It looks like we may want to re-wrap this paragraph after tweaking the wording on the first line. Thanks, Taylor
On Wed, Oct 16, 2024 at 4:46 PM Taylor Blau <me@ttaylorr.com> wrote: > On Tue, Oct 15, 2024 at 09:03:10PM +0200, kristofferhaugsbakk@fastmail.com wrote: > > -With `-d` flag, it deletes the named <ref> after verifying it > > +With `-d`, it deletes the named <ref> after verifying it > > still contains <old-oid>. > > It looks like we may want to re-wrap this paragraph after tweaking the > wording on the first line. I think we typically avoid rewrapping after minor edits like this since rewrapping introduces unnecessary noise which makes it more difficult for reviewers to identify the important (actual) change.
On Wed, Oct 16, 2024 at 06:08:05PM -0400, Eric Sunshine wrote: > On Wed, Oct 16, 2024 at 4:46 PM Taylor Blau <me@ttaylorr.com> wrote: > > On Tue, Oct 15, 2024 at 09:03:10PM +0200, kristofferhaugsbakk@fastmail.com wrote: > > > -With `-d` flag, it deletes the named <ref> after verifying it > > > +With `-d`, it deletes the named <ref> after verifying it > > > still contains <old-oid>. > > > > It looks like we may want to re-wrap this paragraph after tweaking the > > wording on the first line. > > I think we typically avoid rewrapping after minor edits like this > since rewrapping introduces unnecessary noise which makes it more > difficult for reviewers to identify the important (actual) change. I have done it in the past myself, since I often find the result of re-wrapping much nicer to read. But I see what you are saying, and certainly don't feel strongly. Thanks, Taylor
On Thu, Oct 17, 2024, at 00:09, Taylor Blau wrote: > On Wed, Oct 16, 2024 at 06:08:05PM -0400, Eric Sunshine wrote: >> On Wed, Oct 16, 2024 at 4:46 PM Taylor Blau <me@ttaylorr.com> wrote: >> > On Tue, Oct 15, 2024 at 09:03:10PM +0200, kristofferhaugsbakk@fastmail.com wrote: >> > > -With `-d` flag, it deletes the named <ref> after verifying it >> > > +With `-d`, it deletes the named <ref> after verifying it >> > > still contains <old-oid>. >> > >> > It looks like we may want to re-wrap this paragraph after tweaking the >> > wording on the first line. >> >> I think we typically avoid rewrapping after minor edits like this >> since rewrapping introduces unnecessary noise which makes it more >> difficult for reviewers to identify the important (actual) change. I was skeptical at first. But I saw that this line is only 55 characters long. So I think (like Taylor) that rewrap is in order. What if I make a commit with just that word drop and then an immediate fixup! commit which wraps the paragraph? That way the review is still straightforward. And hopefully the integration part is not complicated further. > > I have done it in the past myself, since I often find the result of > re-wrapping much nicer to read. But I see what you are saying, and > certainly don't feel strongly. > > Thanks, > Taylor
On Thu, Oct 17, 2024 at 11:30 AM Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com> wrote: > On Thu, Oct 17, 2024, at 00:09, Taylor Blau wrote: > > On Wed, Oct 16, 2024 at 06:08:05PM -0400, Eric Sunshine wrote: > >> I think we typically avoid rewrapping after minor edits like this > >> since rewrapping introduces unnecessary noise which makes it more > >> difficult for reviewers to identify the important (actual) change. > > I was skeptical at first. But I saw that this line is only 55 > characters long. So I think (like Taylor) that rewrap is in order. > > What if I make a commit with just that word drop and then an immediate > fixup! commit which wraps the paragraph? That way the review is still > straightforward. And hopefully the integration part is not complicated > further. Don't bother. That's even more work for yourself, for reviewers, and for the integrator, and it increases the cognitive load for everyone. There are far fewer reviewers than there are people submitting patches to this project, so it is helpful for submitters to do what they can to make life easier for reviewers, and foregoing re-wrapping of lines, in general, is one such way to do so. However, this is such a minor change that it isn't going to matter one way or the other, especially if Taylor, as interim maintainer, is willing to accept the extra noise caused by re-wrapping.
On Thu, Oct 17, 2024 at 12:31:26PM -0400, Eric Sunshine wrote: > On Thu, Oct 17, 2024 at 11:30 AM Kristoffer Haugsbakk > <kristofferhaugsbakk@fastmail.com> wrote: > > On Thu, Oct 17, 2024, at 00:09, Taylor Blau wrote: > > > On Wed, Oct 16, 2024 at 06:08:05PM -0400, Eric Sunshine wrote: > > >> I think we typically avoid rewrapping after minor edits like this > > >> since rewrapping introduces unnecessary noise which makes it more > > >> difficult for reviewers to identify the important (actual) change. > > > > I was skeptical at first. But I saw that this line is only 55 > > characters long. So I think (like Taylor) that rewrap is in order. > > > > What if I make a commit with just that word drop and then an immediate > > fixup! commit which wraps the paragraph? That way the review is still > > straightforward. And hopefully the integration part is not complicated > > further. > > Don't bother. That's even more work for yourself, for reviewers, and > for the integrator, and it increases the cognitive load for everyone. > > There are far fewer reviewers than there are people submitting patches > to this project, so it is helpful for submitters to do what they can > to make life easier for reviewers, and foregoing re-wrapping of lines, > in general, is one such way to do so. However, this is such a minor > change that it isn't going to matter one way or the other, especially > if Taylor, as interim maintainer, is willing to accept the extra noise > caused by re-wrapping. I agree with everything Eric wrote here. If you want to send a new round that re-wraps the text, please do so in this patch (that is, make patch 1/6 in your new round apply the changes from this version *and* rewrap the containing paragraph). Please do not send fixup! patches or other such things. Thanks, Taylor
diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt index afcf33cf608..fe5967234e9 100644 --- a/Documentation/git-update-ref.txt +++ b/Documentation/git-update-ref.txt @@ -55,7 +55,7 @@ for reading but not for writing (so we'll never write through a ref symlink to some other tree, if you have copied a whole archive by creating a symlink tree). -With `-d` flag, it deletes the named <ref> after verifying it +With `-d`, it deletes the named <ref> after verifying it still contains <old-oid>. With `--stdin`, update-ref reads instructions from standard input and