From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] CodingGuidelines: document NEEDSWORK comments
Date: Thu, 12 Feb 2026 08:10:47 +0100 [thread overview]
Message-ID: <aY1892Rzp1bQsLoW@pks.im> (raw)
In-Reply-To: <xmqqms1ft7il.fsf@gitster.g>
On Wed, Feb 11, 2026 at 11:17:06AM -0800, Junio C Hamano wrote:
> We often say things like /* NEEDSWORK: further _do_ _this_ */ in
> comments, but it is a short-hand to say "We might later want to do
> this. We might not. We do not have to decide it right now at this
> moment in the commit this comment was added. If somebody is
> inclined to work in this area further, the first thing they need to
> do is to figure out if it truly makes sense to do so, before blindly
> doing it.
>
> This seems to have never been documented. Do so now.
I noticed recently that there have been multiple patch series that
blindly turn such NEEDSWORK comments into code. But I agree with you,
the first and most important thing that such an author would need to
worry about is whether the comment still applies, and what the
ramifications of it are.
I almost feel as if NEEDSWORK is a bit of a misnomer, and that something
like NEEDSTHOUGHTS would be a much better fit. But I don't have any
intent to change that throughout our code base right now.
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index df72fe0177..b358d6bfb8 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -33,6 +33,15 @@ Git in general, a few rough rules are:
> achieve and why the changes were necessary (more on this in the
> accompanying SubmittingPatches document).
>
> + - A label "NEEDSWORK:" followed by description of the things to be
> + done is a way to leave in-code comments to document design
> + decisions yet to be made. 80% of the work to resolve a NEEDSWORK
> + comment is to decide if it makes sense to do so. It can be a very
> + valid change to remove an existing NEEDSWORK comment without doing
> + anything else, with the commit log message describing a good
> + argument why it does not make sense to do the thing the NEEDSWORK
> + comment mentioned.
Documenting is a good first step, but I have to wonder whether such
authors would even discover this. But even if not, it means that we have
an easy place to point to going forward.
Thanks!
Patrick
next prev parent reply other threads:[~2026-02-12 7:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-11 19:17 [PATCH] CodingGuidelines: document NEEDSWORK comments Junio C Hamano
2026-02-12 7:10 ` Patrick Steinhardt [this message]
2026-02-12 15:35 ` Junio C Hamano
2026-02-12 21:22 ` [PATCH v2] " Junio C Hamano
2026-02-12 22:22 ` D. Ben Knoble
2026-02-12 22:33 ` Junio C Hamano
2026-02-14 10:19 ` Oswald Buddenhagen
2026-02-14 15:36 ` Junio C Hamano
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=aY1892Rzp1bQsLoW@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox