unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Konstantin Kharlamov <hi-angel@yandex.ru>
Cc: 35062@debbugs.gnu.org
Subject: bug#35062: [PATCH v2] min_cols/rows is always 0, remove noop actions
Date: Sat, 06 Apr 2019 10:09:18 +0300	[thread overview]
Message-ID: <83zhp3a9up.fsf@gnu.org> (raw)
In-Reply-To: <1554502718.25876.0@yandex.ru> (message from Konstantin Kharlamov on Sat, 06 Apr 2019 01:18:38 +0300)

> Date: Sat, 06 Apr 2019 01:18:38 +0300
> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: 35062@debbugs.gnu.org
> 
> >>  * src/gtkutil.c:
> >>      remove "always false" comparison
> >>      remove multiplication by zero, it's a noop anyway
> >>      remove min_cols and min_rows as it's now unused
> >> 
> >>  Fixes LGTM warnings:
> >>      * Comparison is always false because min_cols <= 0.
> >>      * Comparison is always false because min_rows <= 0.
> >>  ---
> >>  v2: remove the min_rows/min_cols whatsoever
> > 
> > Thanks, pushed.  Please in the future write the commit log messages in
> > the ChangeLog-like style, see CONTRIBUTE for the details.  I did that
> > this time, you can look at what I pushed to see an example of that
> > style in action.
> 
> Thanks, so, I'm looking right now to modify the rest of series and 
> resend it here, and… I think I miss something: the only difference of 
> my commit from CONTRIBUTE file is that I forgot to list the functions 
> changed.

That's right.  And also the lack of the bug number, see below.

> I think I misunderstand something, because you completely rewrote the 
> commit messages compared to mine. Titles now are more vague, and for 
> some reason you removed a note that one of patches fixes warnings in 
> the code.

That's just my personal preferences of style, as there's always a
judgment call regarding the description of the changes.  We don't have
to agree about that, and if your log message was in the right format,
I probably wouldn't have bothered changing their text.

I can explain my preference: your text described in a very detailed
form something that was acutely visible from the diffs themselves,
whereas my text only describes the motivation, leaving the rest to the
diffs which speak for themselves.

The part about LGTM warnings didn't seem important enough to me: LGTM
is not a compiler, and without a reference to what it is, the text is
a small riddle in itself.  More importantly, I don't think we need any
justification for removing variables initialized to zero and never
changed.

Again, these are my personal preferences, not something we require
from each contributor.  So if you disagree, you can keep your style,
and only adjust the form.

> And then you also added "Bug#35062", even though there's no 
> bug (the report existence is incidental, it's because bugtracker is 
> used to track patches in Emacs.

References to bug numbers should always be in the log message, because
that allows an easy way of reading relevant discussions recorded by
the bug tracker.  This _is_ in CONTRIBUTE, btw.

> But writing "Bug" will only confuse readers — there's no bug).

"Bug" here doesn't mean a literal bug, it means an issue recorded by
the bug tracker.

Thanks.





  reply	other threads:[~2019-04-06  7:09 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-31 22:36 bug#35062: Patches: trivial cleanups Konstantin Kharlamov
2019-03-31 22:37 ` bug#35062: [PATCH 0/4] Trivial code cleanups Konstantin Kharlamov
2019-03-31 22:37   ` bug#35062: [PATCH 1/4] Remove redundant comparison Konstantin Kharlamov
2019-03-31 22:37   ` bug#35062: [PATCH 2/4] constify a bit of xterm.c Konstantin Kharlamov
2019-03-31 22:37   ` bug#35062: [PATCH 3/4] min_cols/rows is always 0, don't check for it Konstantin Kharlamov
2019-04-01 22:37     ` Noam Postavsky
2019-04-02  0:09       ` Konstantin Kharlamov
2019-04-02 14:46         ` Eli Zaretskii
2019-04-02 20:54           ` Konstantin Kharlamov
2019-04-03  4:45             ` Eli Zaretskii
2019-04-02  0:23       ` bug#35062: [PATCH v2] min_cols/rows is always 0, remove noop actions Konstantin Kharlamov
2019-04-05  7:05         ` Eli Zaretskii
2019-04-05 22:18           ` Konstantin Kharlamov
2019-04-06  7:09             ` Eli Zaretskii [this message]
2019-04-07  2:03               ` Konstantin Kharlamov
2019-04-07  2:45                 ` Eli Zaretskii
2019-04-06  7:25             ` Michael Albinus
2019-03-31 22:37   ` bug#35062: [PATCH 4/4] don't compare unsigned to less-than-zero Konstantin Kharlamov
2019-04-01  4:37   ` bug#35062: [PATCH 0/4] Trivial code cleanups Eli Zaretskii
2019-04-01 13:27     ` Robert Pluim
2019-04-01 13:35       ` Konstantin Kharlamov
2019-04-01 13:41         ` Konstantin Kharlamov
2019-04-01 13:43         ` Robert Pluim
2019-04-01 13:51           ` Konstantin Kharlamov
2019-04-01 14:34       ` Eli Zaretskii
2019-04-01 15:04         ` Robert Pluim
2019-04-01 17:37           ` Eli Zaretskii
2019-04-02 20:49 ` bug#35062: [PATCH] Remove redundant multiplication of ch and cw Konstantin Kharlamov
2019-04-05  7:16   ` Eli Zaretskii
2019-04-07  2:13 ` bug#35062: [PATCH v3 1/3] Remove redundant comparison Konstantin Kharlamov
2019-04-07  2:13   ` bug#35062: [PATCH v3 2/3] constify a bit of xterm.c Konstantin Kharlamov
2019-04-13  8:15     ` Eli Zaretskii
2019-04-13 11:30       ` Konstantin Kharlamov
2019-04-13 11:36         ` Eli Zaretskii
2019-04-20  0:31       ` Paul Eggert
2019-04-20  1:09         ` Konstantin Kharlamov
2019-04-20  1:17           ` Konstantin Kharlamov
2019-04-20  6:53           ` Eli Zaretskii
2019-04-20 10:31             ` Konstantin Kharlamov
2019-04-20 11:01               ` Eli Zaretskii
2019-04-20 11:23                 ` Konstantin Kharlamov
2019-04-20 11:25                   ` Konstantin Kharlamov
2019-04-20 11:47                     ` Konstantin Kharlamov
2019-04-20 11:58                       ` Konstantin Kharlamov
2019-04-20  6:28         ` Eli Zaretskii
2019-04-07  2:13   ` bug#35062: [PATCH v3 3/3] don't compare unsigned to less-than-zero Konstantin Kharlamov
2019-04-13  8:11     ` Eli Zaretskii
2019-04-13  8:06   ` bug#35062: [PATCH v3 1/3] Remove redundant comparison Eli Zaretskii
2019-04-13 18:19     ` Konstantin Kharlamov
2019-04-13 18:24       ` Eli Zaretskii
2019-04-13 18:28         ` Konstantin Kharlamov
2019-04-13 19:19           ` Eli Zaretskii
2019-04-15  3:38             ` Richard Stallman
2019-04-15  6:49               ` Konstantin Kharlamov
2019-04-15 14:32                 ` Eli Zaretskii
2019-04-15 15:01                   ` Konstantin Kharlamov
2019-04-15 15:21                     ` Eli Zaretskii
2019-04-15 17:03                       ` Konstantin Kharlamov
2019-04-15 17:16                         ` Eli Zaretskii
2019-04-15 17:29                           ` Konstantin Kharlamov
2019-04-15 18:21                             ` Eli Zaretskii
2019-04-15 18:14                 ` Richard Stallman
2019-04-15 18:39                   ` Eli Zaretskii
2019-04-15 14:25               ` Eli Zaretskii
2019-04-16 21:27                 ` Richard Stallman
2019-04-17  2:40                   ` Eli Zaretskii
2019-04-17 20:52                     ` Richard Stallman
2019-06-23 18:07 ` bug#35062: Patches: trivial cleanups Lars Ingebrigtsen
2019-06-23 18:34   ` Constantine Kharlamov

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

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83zhp3a9up.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=35062@debbugs.gnu.org \
    --cc=hi-angel@yandex.ru \
    /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.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).