[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

useful commit messages



On Thu, 2014-01-30 at 12:45 -0800, Brian C. Lane wrote:
> Subject: [lorax/master] Install aajohan-comfortaa-fonts (#1047430)
>
> ---
>  share/runtime-install.tmpl | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/share/runtime-install.tmpl b/share/runtime-install.tmpl
> index 889e6c9..e4c0336 100644
> --- a/share/runtime-install.tmpl
> +++ b/share/runtime-install.tmpl
> @@ -127,6 +127,7 @@ installpkg wqy-microhei-fonts
>  installpkg sil-abyssinica-fonts
>  installpkg xorg-x11-fonts-misc
>  installpkg gnome-themes-standard gnome-icon-theme-legacy
> +installpkg aajohan-comfortaa-fonts

Argh. May I make a very strong suggestion for our informal Anaconda
Style Guide?

   >>> COMMIT MESSAGES SHOULD ANSWER THE QUESTION "WHY?" <<<

Because I guarantee you that someday - sooner than you think - somebody
will want to know *why* this package is being installed. And there's
*nothing* here to tell them.

This commit message tells us *what* changed, but that's totally obvious
from the diff. It doesn't need further explanation.

(Yes, "(#XXXXXX)" refers to the bug ID, which explains a bit more, but
there's no guarantee that a future/outside maintainer will know that,
and there's no guarantee that the bug report itself makes any sense, or
is visible to the public, or whatever.)

Having read the bug report I can now tell you that a much better commit
message would have been:

  Install font used in Fedora ransom note SVGs

But would you have been able to guess that from the diff? I wouldn't.

Maybe I'm just traumatized from spending so much time trying to figure
out buildinstall and loader, but please, for everyone's future sanity:

1) Make sure your code explains *why* it's doing what it's doing
2) If you can't explain it in the code, at least mention it in the
commit messages.

THANK YOU FOR YOUR KIND ATTENTION (and sorry for picking on you, Brian)

-w


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]