Re: [PATCH 1/1] branch.c: ammend error messages for die()

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 11, 2023, 17:29 UTC
Message-ID: <xmqqa5spm5ja.fsf@gitster.g>
In-Reply-To: <20231011152424.6957-2-isokenjune@gmail.com>

Isoken June Ibizugbe <isokenjune@gmail.com> writes:

> Subject: Re: [PATCH 1/1] branch.c: ammend error messages for die()

"ammend" is misspelt, but more importantly, it has less information contents than other possible phrases, e.g.,

  Subject: [PATCH 1/1] branch.c: adjust die() messages to coding guidelines

In any case, the title of a commit has insufficient space to describe what the amendment is about, or which exact guideline these messages violate and needs adjustment. This space before your sign-off is where you write it.

Other outreachy candidates have already been given pretty much the same pieces of advice. It may help candidates to learn from the responses given to other candidates. For example, I said the same thing in https://lore.kernel.org/git/xmqqlecbzl5e.fsf@gitster.g/

> Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
> ---
>  builtin/branch.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)

Not a fault of this patch at all, but it is somewhat surprising that we do not break any existing test with this many messages changed. Did you run the test suite before making this commit?

Make it a habit to always do "make test" before committing your work. I am not saying "do not commit what does not pass the tests". What I mean is "be aware of what is still broken (when fixing a bug) or what you broke (when adding a new feature, perhaps as an unintended side effect), before you commit, so that you can describe them in your commit log message".

Thanks.