emacs-devel
[Top][All Lists]
Advanced

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

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


From: Eli Zaretskii
Subject: Re: [Emacs-diffs] master 5515625: ; ChangeLog.2 fixes
Date: Sun, 25 Oct 2015 21:21:02 +0200

> From: Juanma Barranquero <address@hidden>
> Date: Sun, 25 Oct 2015 16:47:15 +0100
> Cc: emacs-devel <address@hidden>
> 
> 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  <address@hidden>

  -       * 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  <address@hidden>

  -       * 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  <address@hidden>

  -       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.



reply via email to

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