From: Eric Sunshine <sunshine@sunshineco.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Ed Maste <emaste@freebsd.org>,
git mailing list <git@vger.kernel.org>,
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
Eric Wong <e@80x24.org>
Subject: Re: [PATCH v3] sparse-checkout: improve OS ls compatibility
Date: Fri, 20 Dec 2019 14:33:04 -0500 [thread overview]
Message-ID: <CAPig+cR64QkBJ8ybMkMiX2CvrgajcGMyG41SMt4mA-VzHyae=A@mail.gmail.com> (raw)
In-Reply-To: <xmqq7e2qajix.fsf@gitster-ct.c.googlers.com>
On Fri, Dec 20, 2019 at 2:23 PM Junio C Hamano <gitster@pobox.com> wrote:
> Ed Maste <emaste@freebsd.org> writes:
> > Ok, I'm also happy if it goes in with no comment; the reason I added
> > it is I could foresee someone coming along in a few years, thinking
> > this is just a strange local implementation of ls, and changing it.
> > But, perhaps we can assume that any such person would check the
> > history before doing so and the comment is not needed.
The in-code comment has sufficient value that I'd like to see it
remain since your concern about someone coming along and wanting to
replace the function with "ls" is a genuine one, and because it saves
people the trouble of having to dig through history in the first
place. And, by "people", I mean that it may save reviewers too since
patch submitters don't always dig through history or don't always
explain _why_ a change is a good idea or valid, which places the
burden on reviewers instead.
> > Indeed, that is more direct, although it's not just FreeBSD ls; this
> > came from 4.2BSD and is probably common to most/all non-GNU ls
> > implementations. In particular, macOS behaves the same way. (Also, the
> > replacement would be even simpler, just "ls $1".)
>
> Good piece of info to include. Final try for the day from me:
>
> # Do not replace this with 'ls "$1"', as "ls" with BSD-lineage
> # enables "-A" by default for root and ends up ...
This looks good to me.
next prev parent reply other threads:[~2019-12-20 19:33 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-19 1:58 [PATCH] sparse-checkout: improve OS ls compatibility Ed Maste
2019-12-19 2:07 ` Derrick Stolee
2019-12-19 2:18 ` Ed Maste
2019-12-19 2:22 ` Derrick Stolee
2019-12-19 2:45 ` Eric Wong
2019-12-19 13:56 ` Derrick Stolee
2019-12-19 16:15 ` Ed Maste
2019-12-19 16:34 ` Derrick Stolee
2019-12-19 18:11 ` Junio C Hamano
2019-12-19 20:56 ` Ed Maste
2019-12-19 22:01 ` Junio C Hamano
2019-12-19 21:45 ` [PATCH v2] " Ed Maste
2019-12-19 22:27 ` Denton Liu
2019-12-20 15:38 ` [PATCH v3] " Ed Maste
2019-12-20 16:05 ` Derrick Stolee
2019-12-20 17:55 ` Junio C Hamano
2019-12-20 17:55 ` Eric Sunshine
2019-12-20 18:15 ` Ed Maste
2019-12-20 18:21 ` Junio C Hamano
2019-12-20 18:34 ` Eric Sunshine
2019-12-20 18:34 ` Ed Maste
2019-12-20 19:23 ` Junio C Hamano
2019-12-20 19:33 ` Eric Sunshine [this message]
2019-12-20 19:41 ` [PATCH v4] " Ed Maste
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPig+cR64QkBJ8ybMkMiX2CvrgajcGMyG41SMt4mA-VzHyae=A@mail.gmail.com' \
--to=sunshine@sunshineco.com \
--cc=e@80x24.org \
--cc=emaste@freebsd.org \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.