Re: [Emacs-diffs] master 5515625: ; ChangeLog.2 fixes

Re: [Emacs-diffs] master 5515625: ; ChangeLog.2 fixes

[Artur Malabarba]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

I'm confused by some of these fixes. The CONTRIBUTE file says not to
punctuate the summary line, and that there should be a blank line between
the summary and the rest.

This seems go against the following change, for example (among others).

> -       * lisp/character-fold.el: Many improvements
> -
> +       * lisp/character-fold.el: Many improvements.
>         (character-fold-search-forward, character-fold-search-backward):

[-- Attachment #2: Type: text/html, Size: 551 bytes --]

Re: [Emacs-diffs] master 5515625: ; ChangeLog.2 fixes

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]

Hi.

The CONTRIBUTE file talks about the commit log messages. We generate the
ChangeLog from the commit messages, but it is its own thing, and I think
that making it more readable, removing redundancies, etc. is good. They
mostly have different targets; we developers are now much more likely to
use git log or gitk than go fishing into ChangeLogs, while people who
downloads a source tarball only has the ChangeLogs. That's why I spend time
cleaning them.

As for that particular example, I've mostly respected cases of "one line
description non-ending in period", but when the one-line description is of
the type

    * file (blabla): some change"

I usually change it to

    * dir/file (blabla): Some change.

for coherence with other entries that do not happen to be the one-line
description of a commit.

I also remove (here and in a patch I'm working on) the one-line description
in cases like:

   Do XXX in function YYY of file ZZZ

   * ZZZ: Do XXX in function YYY.

or

   Do XXX in function YYY of file ZZZ

   * ZZZ(YYY): Do XXX.

but I've left one-line descriptions untouched when they add information or
are formulated differently.

That said, I do these changes that I feel make ChangeLog better, but anyone
is of course entitled to do the same or revert my changes. Or, if there's
some consensus that some of these changes are wrong, I'll adapt to whatever
is preferred.

Thanks,

    Juanma

[-- Attachment #2: Type: text/html, Size: 1905 bytes --]

Re: [Emacs-diffs] master 5515625: ; ChangeLog.2 fixes

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Sun, 25 Oct 2015 16:47:15 +0100
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> The CONTRIBUTE file talks about the commit log messages. We generate the
> ChangeLog from the commit messages, but it is its own thing, and I think that
> making it more readable, removing redundancies, etc. is good. They mostly have
> different targets; we developers are now much more likely to use git log or
> gitk than go fishing into ChangeLogs, while people who downloads a source
> tarball only has the ChangeLogs. That's why I spend time cleaning them.

Thank you for your work.

> As for that particular example, I've mostly respected cases of "one line
> description non-ending in period", but when the one-line description is of the
> type
> 
> * file (blabla): some change"
> 
> I usually change it to
> 
> * dir/file (blabla): Some change.
> 
> for coherence with other entries that do not happen to be the one-line
> description of a commit.

That's correct.  Personally, I think the commit messages themselves
could have the period in this special case, but that's just MO.

> I also remove (here and in a patch I'm working on) the one-line description in
> cases like:
> 
> Do XXX in function YYY of file ZZZ
> 
> * ZZZ: Do XXX in function YYY.
> 
> or
> 
> Do XXX in function YYY of file ZZZ
> 
> * ZZZ(YYY): Do XXX.
> 
> but I've left one-line descriptions untouched when they add information or are
> formulated differently.

This is borderline, and you might as well leave those untouched.

> That said, I do these changes that I feel make ChangeLog better, but anyone is
> of course entitled to do the same or revert my changes. Or, if there's some
> consensus that some of these changes are wrong, I'll adapt to whatever is
> preferred.

These are issues of style, so there are no hard rules.  Back when we
maintained ChangeLog files by hand the style was not really uniform,
either.

Some more comments to your changes:

   2015-10-24  Artur Malabarba  <bruce.connor.am@gmail.com>

  -       * lisp/character-fold.el: Many improvements
  -
  +       * lisp/character-fold.el: Many improvements.
	  (character-fold-search-forward, character-fold-search-backward):
  -       New command
  +       New command.
	  (character-fold-to-regexp): Remove lax-whitespace hack.
	  (character-fold-search): Remove variable.  Only isearch and
	  query-replace use char-folding, and they both have their own

The "many improvements" part could simply go away, it doesn't add any
useful information.

Also, Artur, I think having part of a ChangeLog style entries in the
header line and the rest in the body is not a very good idea.  Here's
one more example:

   2015-10-24  Artur Malabarba  <bruce.connor.am@gmail.com>

  -       * lisp/isearch.el (search-default-regexp-mode): New variable
  -
  +       * lisp/isearch.el (search-default-regexp-mode): New variable.
	  (isearch-mode): Use it.

I think the commit message should have been as corrected to begin
with, and a header line should say something else, without being
formatted as an entry.

	  * nt/icons/emacs.ico:
  -       * nextstep/Cocoa/Emacs.base/Contents/Resources/Emacs.icns: Use the new
  -       icons.
  +       * nextstep/Cocoa/Emacs.base/Contents/Resources/Emacs.icns:
  +       Use the new icons.

This is okay, but when I see such changes, I always ask myself whether
it's worth the trouble.  Your call.

   2015-10-20  Dmitry Gutov  <dgutov@yandex.ru>

  -       Don't declare vc-exec-after anymore
  -
	  * lisp/vc/vc-svn.el:
	  * lisp/vc/vc-mtn.el:
	  * lisp/vc/vc-hg.el:
	  * lisp/vc/vc-cvs.el:
	  * lisp/vc/vc-git.el:
  -       * lisp/vc/vc-bzr.el: Don't declare vc-exec-after anymore.  Its
  -       usages have been replaced with vc-run-delayed.
  +       * lisp/vc/vc-bzr.el: Don't declare vc-exec-after anymore.
  +       Its usages have been replaced with vc-run-delayed.

Not sure why the header line was deleted, it looks OK to me.

Thanks again for working on this.

Re: [Emacs-diffs] master 5515625: ; ChangeLog.2 fixes

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 2792 bytes --]

On Sun, Oct 25, 2015 at 8:21 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> That's correct.  Personally, I think the commit messages themselves
> could have the period in this special case, but that's just MO.

I agree.

> This is borderline, and you might as well leave those untouched.

The ones I change tend to be like

        Use `unless' to have one fewer `not'
        * lisp/vc/vc-git.el (vc-git-resolve-when-done): Use `unless' to
        have one fewer `not'.

or

        Fix customization type of `even-window-sizes'.
        * lisp/window.el (even-window-sizes): Fix customization type.

where, really, the header doesn't add anything of value (to the ChangeLog).

> These are issues of style, so there are no hard rules.  Back when we
> maintained ChangeLog files by hand the style was not really uniform,
> either.

Yes, I know. I didn't try then, and do not try now, to have absolute
consistency, just to increment it a little ;-)

>   +       * lisp/character-fold.el: Many improvements.
>           (character-fold-search-forward, character-fold-search-backward):
>   -       New command

> The "many improvements" part could simply go away, it doesn't add any
> useful information.

I'd tend to agree, but I'm conservative in removing other people's words
unless they are really redundant (as in the cases above).

> I think the commit message should have been as corrected to begin
> with, and a header line should say something else, without being
> formatted as an entry.

Certainly. The best messages IMHO are those who do *not* format the header
line as an entry. They are usually more informative and less redundant.

> This is okay, but when I see such changes, I always ask myself whether
> it's worth the trouble.  Your call.

Some of these are because I use column markers to see which entries go over
75 or 80 columns, and some because I agree with Stefan that having dangling
words is ugly. Which doesn't mean that I don't leave some of these, when
the alternative is equally ugly, of course...

>    2015-10-20  Dmitry Gutov  <dgutov@yandex.ru>
>
>   -       Don't declare vc-exec-after anymore
>   -
>           * lisp/vc/vc-svn.el:
>           * lisp/vc/vc-mtn.el:
>           * lisp/vc/vc-hg.el:
>           * lisp/vc/vc-cvs.el:
>           * lisp/vc/vc-git.el:
>   -       * lisp/vc/vc-bzr.el: Don't declare vc-exec-after anymore.  Its
>   -       usages have been replaced with vc-run-delayed.
>   +       * lisp/vc/vc-bzr.el: Don't declare vc-exec-after anymore.
>   +       Its usages have been replaced with vc-run-delayed.
>
> Not sure why the header line was deleted, it looks OK to me.

Borderline, but the header is duplicated word-for-word in the entry. I
think I would've left it if the file list were quite long.

Thanks for your comments.

     J

[-- Attachment #2: Type: text/html, Size: 3784 bytes --]

Re: [Emacs-diffs] master 5515625: ; ChangeLog.2 fixes

[Artur Malabarba]

2015-10-25 22:25 GMT+00:00 Juanma Barranquero <lekktu@gmail.com>:
> [...]

Cool. It makes sense now that you've explained it. :-)
Thanks for the effort.

I wonder if we can improve the changelog script to do some of that
stuff automatically. Of course, a lot of that can't be automated, but
the blank-lines and perhaps the periods should be easy.