Page MenuHomeFreeBSD

Emacs ports: no-op to conform to var order and improve consistency
ClosedPublic

Authored by jrm on Sep 4 2019, 7:35 PM.

Details

Summary

PR:
Submitted by:
Reported by:

Test Plan

poudriere testport looks good

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26277
Build 24762: arc lint + arc unit

Event Timeline

Also remove EMACS_VER since it's only used once

Hi,

In D21524#468847, @jrm wrote:

Also remove EMACS_VER since it's only used once

I think you meant EMACS_REV here ?

editors/emacs-devel/Makefile
200

Also, as I see that you have replaced all occurences EMACS_VER with DISTVERSION:R, which IMHO looks ugly. If you'd rather see Emacs version at the top of the file (easy to locate which sounds fair to me), then maybe initialize EMACS_VER as ${DISTVERSION:R}, while keeping everything that includes EMACS_VER as is ?

ashish requested changes to this revision.Sep 5 2019, 9:53 PM
This revision now requires changes to proceed.Sep 5 2019, 9:53 PM

Hi,

In D21524#468847, @jrm wrote:

Also remove EMACS_VER since it's only used once

I think you meant EMACS_REV here ?

Yup.

As an aside, the garbage collection bug was not completely fixed and Paul E. submitted another patch. See messages #72 and later at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37006

editors/emacs-devel/Makefile
200

My motivation was to reduce the diff between editors/emacs and editors/emacs-devel. In editors/emacs DISTVERSION is used because it would be the same as EMACS_VER. In editors/emacs-devel DISTVERSION is the same as EMACS_VER, except without the revision date, which is what ${DISTVERSION:R} sort of means. Even with these changes, a diff would still flag these lines as different, just less different, plus there would be one less line in the diff for defining EMACS_VER.

Putting my silly obsessions aside, it's not so important to me either way, so if this motivation does not seem useful, I will follow your suggestion.

In D21524#469200, @jrm wrote:

Hi,

In D21524#468847, @jrm wrote:

Also remove EMACS_VER since it's only used once

I think you meant EMACS_REV here ?

Yup.

As an aside, the garbage collection bug was not completely fixed and Paul E. submitted another patch. See messages #72 and later at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37006

Thanks for working on it.

editors/emacs-devel/Makefile
200

I haven't looked at these changes from the that perspective, and do not think it's a silly obsession. I think from maintenance point of view, your initiative is a good one, less chances of human errors.

I made this diff implying what I think should happen, w.r.t. EMACS_VER. And also created a diff to illustrate differences between emacs, and emacs-devel ports.

Let me know what you think.

editors/emacs-devel/Makefile
200

Sorry, ignore the diffs in previous comment, right diffs are below:

This is a reasonable approach, however I don't see the value in defining EMACS_REV since we are essentially doing VAR1=some_value then the only reference to VAR1 is VAR2=VAR1. I sort of feel the same about EMACS_VER, since it holds the same value as DISTVERSION in editors/emacs and almost the same value in editors/emacs-devel. It seems to add an extra layer (What's the value of EMACS_VER? It's DISTVERSION. What's the value of DISTVERSION? It's 26.3).

Having said this, I don't think it makes that much difference either way, and I yield to your seniority as Emacs maintainer here. :)

EDIT: Fix spelling of EMACS_REV.

As discussed on Slack, your changes are okay.

Thanks!

This revision is now accepted and ready to land.Sep 7 2019, 2:07 AM