Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

[Glenn Morris]

Hi,

Please add a ChangeLog entry at the appropriate date for this and other
recent changes (to test). Also, defcustom with non-trivial changes
should get :version bumps (don't know if this is non-trivial).

Seems lke test ruby-exit!-font-lock should have expected-result :failed
for now, else it causes a coverage failure on hydra, which could mask
the introduction of real problems.

See eg http://hydra.nixos.org/build/6852191

Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

[Bozhidar Batsov]

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

Seems odd to include such trivial changes in the changelog, as nobody is
likely to be interested in reading about them. This particular defcustom
was recently introduced, that's why I considered altering its default an
extremely tiny change.

As for the tests - I'll change the state of the problematic test to have
expected-result :failed. Sorry about that. Btw, what's the value of listing
test changes in the changelog? Seems to me that pretty few people would be
interested in reading about such changes.


On 21 November 2013 17:41, Glenn Morris <rgm@gnu.org> wrote:

>
> Hi,
>
> Please add a ChangeLog entry at the appropriate date for this and other
> recent changes (to test). Also, defcustom with non-trivial changes
> should get :version bumps (don't know if this is non-trivial).
>
> Seems lke test ruby-exit!-font-lock should have expected-result :failed
> for now, else it causes a coverage failure on hydra, which could mask
> the introduction of real problems.
>
> See eg http://hydra.nixos.org/build/6852191
>

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

Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

[Glenn Morris]

Bozhidar Batsov wrote:

> Seems odd to include such trivial changes in the changelog, as nobody is
> likely to be interested in reading about them.

It is the Emacs convention to record everything (essentially) in
ChangeLogs. It's no extra work. Eg, use `C-x 4 a' to create the
ChangeLog entry, then use log-edit-insert-changelog to create the VCS
log entry from the ChangeLog.

> This particular defcustom was recently introduced,

Then it should definitely have a :version tag. :)

(BTW, there isn't even a ChangeLog entry for the introduction.)

Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

[Glenn Morris]

Glenn Morris wrote:

> Then it should definitely have a :version tag. :)

And to further the pedantry, the first line of the doc-string should be
a complete sentence. :)

Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

[Bozhidar Batsov]

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

Done. I saw you had already fixed the test expectation. Thanks for the
feedback!


On 22 November 2013 01:17, Glenn Morris <rgm@gnu.org> wrote:

> Glenn Morris wrote:
>
> > Then it should definitely have a :version tag. :)
>
> And to further the pedantry, the first line of the doc-string should be
> a complete sentence. :)
>

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

Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

[Glenn Morris]

Thanks for adding a test/ChangeLog entry.
Do please list the names of the tests in the entry.
ChangeLog entries should name every function, variable that they touch.
I notice this did not happen eg in
http://lists.gnu.org/archive/html/emacs-diffs/2013-11/msg00216.html
which changed an option, added several new functions, and modified
another, but only mentioned the last in the ChangeLog.

Similarly
http://lists.gnu.org/archive/html/emacs-diffs/2013-11/msg00150.html
does not list the two new options as entities in their own right.

Please could you review all your past changes with an eye to that,
and improve the ChangeLogs where necessary?

Also, maybe you want to add a NEWS entry about those new options.
This need be nothing more than:

  ** New Ruby mode options: foo, bar.

in the "Changes in Specialized Modes and Packages in Emacs 24.4" section.

Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

[Bozhidar Batsov]

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

On 23 November 2013 04:37, Glenn Morris <rgm@gnu.org> wrote:

>
> Thanks for adding a test/ChangeLog entry.
> Do please list the names of the tests in the entry.
>

Will do.


> ChangeLog entries should name every function, variable that they touch.
> I notice this did not happen eg in
> http://lists.gnu.org/archive/html/emacs-diffs/2013-11/msg00216.html
> which changed an option, added several new functions, and modified
> another, but only mentioned the last in the ChangeLog.
>

Since the new functions were extracted from the modified function I wasn't
sure if they needed mentioning.
Basically in a situation like this I should list all symbols like this
(sym1, sym2...): and write a unified summary regarding
all of them, right?


>
> Similarly
> http://lists.gnu.org/archive/html/emacs-diffs/2013-11/msg00150.html
> does not list the two new options as entities in their own right.
>
> Please could you review all your past changes with an eye to that,
> and improve the ChangeLogs where necessary?
>

Sure.


>
> Also, maybe you want to add a NEWS entry about those new options.
> This need be nothing more than:
>
>   ** New Ruby mode options: foo, bar.
>

Will do.


>
> in the "Changes in Specialized Modes and Packages in Emacs 24.4" section.
>

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

Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

[Glenn Morris]

Bozhidar Batsov wrote:

> Since the new functions were extracted from the modified function I wasn't
> sure if they needed mentioning.
> Basically in a situation like this I should list all symbols like this
> (sym1, sym2...): and write a unified summary regarding
> all of them, right?

I'd write something like:

(newfunc1, newfunc2): New functions, extracted from oldfunc.
(oldfunc): Split into several functions, to <do new stuff>.

Re: trunk r115158: * lisp/progmodes/ruby-mode.el (ruby-custom-encoding-magic-comment-template):

[Bozhidar Batsov]

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

Thanks for the suggestion. I'll do the updates today.


On 25 November 2013 21:40, Glenn Morris <rgm@gnu.org> wrote:

> Bozhidar Batsov wrote:
>
> > Since the new functions were extracted from the modified function I
> wasn't
> > sure if they needed mentioning.
> > Basically in a situation like this I should list all symbols like this
> > (sym1, sym2...): and write a unified summary regarding
> > all of them, right?
>
> I'd write something like:
>
> (newfunc1, newfunc2): New functions, extracted from oldfunc.
> (oldfunc): Split into several functions, to <do new stuff>.
>

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