bug-gnu-emacs
[Top][All Lists]
Advanced

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

bug#35062: [PATCH v2] min_cols/rows is always 0, remove noop actions


From: Eli Zaretskii
Subject: bug#35062: [PATCH v2] min_cols/rows is always 0, remove noop actions
Date: Sat, 06 Apr 2019 10:09:18 +0300

> Date: Sat, 06 Apr 2019 01:18:38 +0300
> From: Konstantin Kharlamov <address@hidden>
> Cc: address@hidden
> 
> >>  * 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 via email to

[Prev in Thread] Current Thread [Next in Thread]