Thread (50 messages) 50 messages, 7 authors, 2022-11-22

Re: several messages

From: Eric Sunshine <hidden>
Date: 2022-09-02 18:16:39

On Fri, Sep 2, 2022 at 8:42 AM Johannes Schindelin
[off-list ref] wrote:
On Thu, 1 Sep 2022, Eric Sunshine via GitGitGadget wrote:
quoted
 t/chainlint.pl                                | 730 ++++++++++++++++++
 t/chainlint.sed                               | 399 ----------
 t/chainlint/blank-line-before-esac.expect     |  18 +
 t/chainlint/blank-line-before-esac.test       |  19 +
 ...
This looks like it was a lot of work. And that it would be a lot of work
to review, too, and certainly even more work to maintain.

Are we really sure that we want to burden the Git project with this much
stuff that is not actually related to Git's core functionality?

It would be one thing if we could use a well-maintained third-party tool
to do this job. But adding this to our plate? I hope we can avoid that.
I understand your concerns about review and maintenance burden, and
you're not the first to make such observations; when chainlint.sed was
submitted, it was greeted with similar concerns[1,2], all very
understandable. The key takeaway[3] from that conversation, though,
was that, unlike user-facing features which must be reviewed in detail
and maintained in perpetuity, this is a mere developer aid which can
be easily ejected from the project if it ever becomes a maintenance
burden or shows itself to be unreliable. Potential maintenance burden
aside, a very real benefit of such a tool is that it should help
prevent bugs from slipping into the project going forward[4], which is
indeed the aim of all our developer-focused aids.

In more practical terms, despite initial concerns, in the 4+ years
since its introduction, the maintenance cost of chainlint.sed has been
nearly zero. Very early on, there was a report[5] that chainlint.sed
was showing a false-positive in a `contrib` test script; the developer
quickly responded with a fix[6]. The only other maintenance issues
were a couple dead-simple changes[7,8] to shorten "labels" to support
older versions of `sed`. (As for the chainlint self-tests, the
maintenance cost has been exactly zero). My hope is that chainlint.pl
should have a similar track-record, but it can easily be dropped from
the project if not.

[1]: https://lore.kernel.org/git/xmqqk1q11mkj.fsf@gitster-ct.c.googlers.com/ (local)
[2]: https://lore.kernel.org/git/20180712165608.GA10515@sigill.intra.peff.net/ (local)
[3]: https://lore.kernel.org/git/CAPig+cRmAkiYqFXwRAkQALDoOo-79r2iAumdEJEZhBnETvL-fw@mail.gmail.com/ (local)
[4]: https://lore.kernel.org/git/xmqqin5kw7q3.fsf@gitster-ct.c.googlers.com/ (local)
[5]: https://lore.kernel.org/git/20180730181356.GA156463@aiede.svl.corp.google.com/ (local)
[6]: https://lore.kernel.org/git/20180807082135.60913-1-sunshine@sunshineco.com/ (local)
[7]: https://lore.kernel.org/git/20180824152016.20286-5-avarab@gmail.com/ (local)
[8]: https://lore.kernel.org/git/d15ed626de65c51ef2ba31020eeb2111fb8e091f.1596675905.git.gitgitgadget@gmail.com/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help