bug#28637: [PATCH] Display commit in package description, if available

bug#28637: [PATCH] Display commit in package description, if available

[David Glasser]

MELPA includes a :commit field in its
packages (https://github.com/melpa/package-build/pull/6). You can use
this to tell if MELPA has processed a recently-merged change.  This
commit adds that metadata to the package description buffer.

* lisp/emacs-lisp/package.el: Display commit in package description
---
 lisp/emacs-lisp/package.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 8b101c1323..dd05c70dc8 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2260,6 +2260,7 @@ Otherwise no newline is inserted."
          (archive (if desc (package-desc-archive desc)))
          (extras (and desc (package-desc-extras desc)))
          (homepage (cdr (assoc :url extras)))
+         (commit (cdr (assoc :commit extras)))
          (keywords (if desc (package-desc--keywords desc)))
          (built-in (eq pkg-dir 'builtin))
          (installable (and archive (not built-in)))
@@ -2332,6 +2333,8 @@ Otherwise no newline is inserted."
     (and version
          (package--print-help-section "Version"
            (package-version-join version)))
+    (when commit
+      (package--print-help-section "Commit" commit))
     (when desc
       (package--print-help-section "Summary"
         (package-desc-summary desc)))
--
2.14.1

bug#28637: Display commit in package description, if available

[David Glasser]

Can I confirm that this went through successfully? Should I be
bringing up this patch on a different email list?

bug#28637: Display commit in package description, if available

[Eli Zaretskii]

> From: David Glasser <glasser@davidglasser.net>
> Date: Tue, 10 Oct 2017 12:35:58 -0700
> 
> Can I confirm that this went through successfully?

AFAICT, no one has reviewed this yet.

> Should I be bringing up this patch on a different email list?

This is the right place, just keep pinging until the patch is
reviewed.

Thanks.

bug#28637: Display commit in package description, if available

[David Glasser]

OK, sure!

I think this is a pretty minimal-impact change which makes the
experience of using melpa a lot better. I frequently send PRs to
packages that I fetch via melpa, and once I get the "merged"
notification, I want to be able to tell whether or not my change is
available in the melpa build.

bug#28637: [PATCH] Display commit in package description, if available

[Noam Postavsky]

David Glasser <glasser@davidglasser.net> writes:

> MELPA includes a :commit field in its
> packages (https://github.com/melpa/package-build/pull/6). You can use
> this to tell if MELPA has processed a recently-merged change.  This
> commit adds that metadata to the package description buffer.

Code looks fine, but could you post a "before and after" picture please?
I'm wondering if it might make sense to abbreviate the hash.

> * lisp/emacs-lisp/package.el: Display commit in package description

I think you should have a period at the end of sentence though.

bug#28637: [PATCH] Display commit in package description, if available

[David Glasser]

On Tue, Oct 10, 2017 at 3:40 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> David Glasser <glasser@davidglasser.net> writes:
>
>> MELPA includes a :commit field in its
>> packages (https://github.com/melpa/package-build/pull/6). You can use
>> this to tell if MELPA has processed a recently-merged change.  This
>> commit adds that metadata to the package description buffer.
>
> Code looks fine, but could you post a "before and after" picture please?
> I'm wondering if it might make sense to abbreviate the hash.

Sure! Here's a randomly chosen melpa package's *Help* page:

```
archive-rpm is a new package.

     Status: New from melpa -- Install
    Archive: melpa
    Version: 20171005.1548
     Commit: 830158cfb3b43c85cfcb4bd5b92d4457d015c80a
    Summary: RPM and CPIO support for archive-mode
   Requires: emacs-24.4

This module adds support for RPM archives to archive-mode.

RPM files consist of metadata plus a compressed CPIO archive, so
this module relies on `archive-cpio'.
```

This is the "after" picture; the "before" picture lacks the Commit line.

Abbreviating the hash might be nice, but it seems like maybe that should be
the job of the code that creates the metadata (ie in melpa's package builder)
rather than the code that displays it.


>> * lisp/emacs-lisp/package.el: Display commit in package description
>
> I think you should have a period at the end of sentence though.

OK, a new version of the patch is below (is this the right way to send
a new version of the patch? I am not used to mail-based git
workflows):

From a4ebfa2ed35a620e4754399da8181caba13a1eb9 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@davidglasser.net>
Date: Thu, 28 Sep 2017 14:00:04 -0700
Subject: [PATCH] Display commit in package description, if available

MELPA includes a :commit field in its
packages (https://github.com/melpa/package-build/pull/6). You can use
this to tell if MELPA has processed a recently-merged change.  This
commit adds that metadata to the package description buffer.

* lisp/emacs-lisp/package.el: Display commit in package description.
---
 lisp/emacs-lisp/package.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 8b101c1323..dd05c70dc8 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2260,6 +2260,7 @@ Otherwise no newline is inserted."
          (archive (if desc (package-desc-archive desc)))
          (extras (and desc (package-desc-extras desc)))
          (homepage (cdr (assoc :url extras)))
+         (commit (cdr (assoc :commit extras)))
          (keywords (if desc (package-desc--keywords desc)))
          (built-in (eq pkg-dir 'builtin))
          (installable (and archive (not built-in)))
@@ -2332,6 +2333,8 @@ Otherwise no newline is inserted."
     (and version
          (package--print-help-section "Version"
            (package-version-join version)))
+    (when commit
+      (package--print-help-section "Commit" commit))
     (when desc
       (package--print-help-section "Summary"
         (package-desc-summary desc)))

bug#28637: [PATCH] Display commit in package description, if available

[Noam Postavsky]

David Glasser <glasser@davidglasser.net> writes:

> Abbreviating the hash might be nice, but it seems like maybe that should be
> the job of the code that creates the metadata (ie in melpa's package builder)
> rather than the code that displays it.

Hmm, I'm not so sure about that, but looking at the result now I think
there's no need to abbreviate it anyway.

I think this should be okay to go to emacs-26; it's not strictly
speaking a bug fix, but it's obviously safe and this info is often
useful for package maintainers fielding bug reports, so it would be good
to have it more widely available.  Eli?

> OK, a new version of the patch is below (is this the right way to send
> a new version of the patch? I am not used to mail-based git
> workflows):

Yup, that's fine.  You can also use the -v option to git-format-patch in
order to make it more obvious that the patch is new.  For small stuff
like this, it's not really important though.

I suppose you haven't assigned copyright for Emacs?  No problem if not,
the patch is small enough to go in anyway, we just have to mark it.

bug#28637: [PATCH] Display commit in package description, if available

[David Glasser]

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

On Oct 10, 2017 6:16 PM, "Noam Postavsky" <npostavs@users.sourceforge.net>
wrote:

I suppose you haven't assigned copyright for Emacs?  No problem if not,
the patch is small enough to go in anyway, we just have to mark it.


I don't believe I have.

--dave

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

bug#28637: [PATCH] Display commit in package description, if available

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Tue, 10 Oct 2017 21:16:36 -0400
> Cc: 28637@debbugs.gnu.org
> 
> I think this should be okay to go to emacs-26; it's not strictly
> speaking a bug fix, but it's obviously safe and this info is often
> useful for package maintainers fielding bug reports, so it would be good
> to have it more widely available.  Eli?

Are we sure no feature parses this output, and might be confused by
the extra line?

bug#28637: [PATCH] Display commit in package description, if available

[Noam Postavsky]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Tue, 10 Oct 2017 21:16:36 -0400
>> Cc: 28637@debbugs.gnu.org
>> 
>> I think this should be okay to go to emacs-26; it's not strictly
>> speaking a bug fix, but it's obviously safe and this info is often
>> useful for package maintainers fielding bug reports, so it would be good
>> to have it more widely available.  Eli?
>
> Are we sure no feature parses this output, and might be confused by
> the extra line?

As far as I can tell, it's only used for the *Help* buffer, and nothing
parses that.

bug#28637: [PATCH] Display commit in package description, if available

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: glasser@davidglasser.net,  28637@debbugs.gnu.org
> Date: Wed, 11 Oct 2017 08:32:43 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Noam Postavsky <npostavs@users.sourceforge.net>
> >> Date: Tue, 10 Oct 2017 21:16:36 -0400
> >> Cc: 28637@debbugs.gnu.org
> >> 
> >> I think this should be okay to go to emacs-26; it's not strictly
> >> speaking a bug fix, but it's obviously safe and this info is often
> >> useful for package maintainers fielding bug reports, so it would be good
> >> to have it more widely available.  Eli?
> >
> > Are we sure no feature parses this output, and might be confused by
> > the extra line?
> 
> As far as I can tell, it's only used for the *Help* buffer, and nothing
> parses that.

Then it could be okay for emacs-26, if its addition is deemed
important enough.

bug#28637: [PATCH] Display commit in package description, if available

[Noam Postavsky]

Eli Zaretskii <eliz@gnu.org> writes:

> Then it could be okay for emacs-26, if its addition is deemed
> important enough.

From my perspective as a package maintainer, it would be very useful to
get the commit hash in bug reports.  Getting a timestamp in place of a
version from MELPA users has been a pretty long-standing source of minor
irritation.  So I would like to have this in Emacs 26 to end the
irritation sooner rather than later.

Obviously it's not vital, but it usefulness vs risk ratio seems to me
high enough to be worth it.

bug#28637: [PATCH] Display commit in package description, if available

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: glasser@davidglasser.net,  28637@debbugs.gnu.org
> Date: Mon, 23 Oct 2017 19:58:32 -0400
> 
> >From my perspective as a package maintainer, it would be very useful to
> get the commit hash in bug reports.  Getting a timestamp in place of a
> version from MELPA users has been a pretty long-standing source of minor
> irritation.  So I would like to have this in Emacs 26 to end the
> irritation sooner rather than later.
> 
> Obviously it's not vital, but it usefulness vs risk ratio seems to me
> high enough to be worth it.

OK.

bug#28637: [PATCH] Display commit in package description, if available

[Noam Postavsky]

tags 28637 fixed
close 28637 26.1
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Cc: glasser@davidglasser.net,  28637@debbugs.gnu.org
>> Date: Mon, 23 Oct 2017 19:58:32 -0400
>> 
>> >From my perspective as a package maintainer, it would be very useful to
>> get the commit hash in bug reports.  Getting a timestamp in place of a
>> version from MELPA users has been a pretty long-standing source of minor
>> irritation.  So I would like to have this in Emacs 26 to end the
>> irritation sooner rather than later.
>> 
>> Obviously it's not vital, but it usefulness vs risk ratio seems to me
>> high enough to be worth it.
>
> OK.

Pushed to emacs-26.

[1: 0c536a20fb]: 2017-10-24 19:14:30 -0400
  Display commit in package description, if available (Bug#28637)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=0c536a20fb4833bafea1c2a14b9ff2bac2a3ebd8