diff mbox series

[bug#49219] Acknowledgement ([PATCH]: Update emacs-telega.)

Message ID 86h7hm8oth.fsf@163.com
State Accepted
Headers show
Series [bug#49219] Acknowledgement ([PATCH]: Update emacs-telega.) | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Zhu Zihao June 25, 2021, 6:02 a.m. UTC 
Update patches:

Comments

Leo Prikler June 25, 2021, 6:23 a.m. UTC | #1 
Am Freitag, den 25.06.2021, 14:02 +0800 schrieb Zhu Zihao:
> Update patches:
W.r.t. the propagation of ffmpeg and libwebp I don't think we'd want to
force either onto the user (particularly ffmpeg might collide with a
different version they've already installed).  If finding and replacing
all instances of "ffmpeg" with its full path is too broad, perhaps you
could add procedures that find it or replace instances of ffmpeg as a
binary with @ffmpeg@.  I think a variable or procedure called telega-
ffmpeg-command might even be acceptable by upstream and broader Emacs
standards.
If on the other hand you want the user's versions of those packages to
be the one that's used, simply don't propagate it.

Regards,
Leo
Zhu Zihao June 25, 2021, 9:31 a.m. UTC | #2 
Leo Prikler writes:

> Am Freitag, den 25.06.2021, 14:02 +0800 schrieb Zhu Zihao:
>> Update patches:
> W.r.t. the propagation of ffmpeg and libwebp I don't think we'd want to
> force either onto the user (particularly ffmpeg might collide with a
> different version they've already installed).  If finding and replacing
> all instances of "ffmpeg" with its full path is too broad, perhaps you
> could add procedures that find it or replace instances of ffmpeg as a
> binary with @ffmpeg@.  I think a variable or procedure called telega-
> ffmpeg-command might even be acceptable by upstream and broader Emacs
> standards.
> If on the other hand you want the user's versions of those packages to
> be the one that's used, simply don't propagate it.
>
> Regards,
> Leo

Good, I think I can just remove ffmpeg and libwebp, leave the choice to
user.

Another question is, the new version is 0.7.024, which is lesser than
0.7.1. could you give me some suggestions on the version number?
Leo Prikler June 25, 2021, 10:46 a.m. UTC | #3 
Am Freitag, den 25.06.2021, 17:31 +0800 schrieb Zhu Zihao:
> Leo Prikler writes:
> 
> > Am Freitag, den 25.06.2021, 14:02 +0800 schrieb Zhu Zihao:
> > > Update patches:
> > W.r.t. the propagation of ffmpeg and libwebp I don't think we'd
> > want to
> > force either onto the user (particularly ffmpeg might collide with
> > a
> > different version they've already installed).  If finding and
> > replacing
> > all instances of "ffmpeg" with its full path is too broad, perhaps
> > you
> > could add procedures that find it or replace instances of ffmpeg as
> > a
> > binary with @ffmpeg@.  I think a variable or procedure called
> > telega-
> > ffmpeg-command might even be acceptable by upstream and broader
> > Emacs
> > standards.
> > If on the other hand you want the user's versions of those packages
> > to
> > be the one that's used, simply don't propagate it.
> > 
> > Regards,
> > Leo
> 
> Good, I think I can just remove ffmpeg and libwebp, leave the choice
> to
> user.
> 
> Another question is, the new version is 0.7.024, which is lesser than
> 0.7.1. could you give me some suggestions on the version number?
Tough questions, you might want to ask upstream what their rationale is
on the leading 0 and whether they plan to use a version 24 that
supersedes it.  Given that the switch happened between 0.7.15 and
0.7.016, you might look there for indicators, but I'm also somewhat
tempted to just drop that 0.

Regards,
Leo
Zhu Zihao June 26, 2021, 1:16 p.m. UTC | #4 
Leo Prikler writes:

https://github.com/zevlg/telega.el/issues/297

Just ask developer of telega.

Seems that we'd better to let it downgrade :thinking:
Giovanni Biscuolo July 7, 2021, 1:51 p.m. UTC | #5 
Hello Zhu,

thank you for your work on emacs-telega!

...sorry I'm late in this discussion.

Zhu Zihao <all_but_last@163.com> writes:

> https://github.com/zevlg/telega.el/issues/297
>
> Just ask developer of telega.
>
> Seems that we'd better to let it downgrade :thinking:

for archival purposes, this is the comment [1] by Zajcev Evgeny:

--8<---------------cut here---------------start------------->8---

0.7.0XX is for stable telega from "releases" branch

0.7.XX is for bleading edge telega from "master" branch

We need a way to distinguish them, we use leading 0 right now

--8<---------------cut here---------------end--------------->8---

Also, the upstream README.md states:

--8<---------------cut here---------------start------------->8---

This project is developed on two primary branches. The 'releases' branch
is kept in compatibility with TDLib major releases. The 'master' branch
is for developmental purposes, and utilizes unstable features in TDLib.

--8<---------------cut here---------------end--------------->8---

As a general rule (AFAIU) Guix packages usually are based on stable
releases so probably we should always use <major>.<minor>.0<N> from now
on: WDYT?

In this case I'd add a comment before (version ...) to clarify this
numbering scheme, something like:

--8<---------------cut here---------------start------------->8---

(define-public emacs-telega-server
  (package
    (name "emacs-telega-server")
    ;; 0.7.0XX is for stable telega from "releases" branch
    ;; that is kept in compatibility with TDLib major releases
    (version "0.7.024")

--8<---------------cut here---------------end--------------->8---

Also, if you think it's useful in the future we could also add a
"emacs-telega-server-unstable" for testing bleading edge TDLib features.

WDYT?

I'll give your patch series a try if I find some time this week.

Thanks! Gio'


[1] https://github.com/zevlg/telega.el/issues/297#issuecomment-868992025
Zhu Zihao July 9, 2021, 10:36 a.m. UTC | #6 
Giovanni Biscuolo writes:

>
> Also, if you think it's useful in the future we could also add a
> "emacs-telega-server-unstable" for testing bleading edge TDLib features.
>
> WDYT?
>
> I'll give your patch series a try if I find some time this week.
>
> Thanks! Gio'
>
>
> [1] https://github.com/zevlg/telega.el/issues/297#issuecomment-868992025

I'm OK with it, maybe rename it to `emacs-telega-server-next`?

But I'm not sure Guix maintainer would accept it or not.
Leo Prikler July 9, 2021, 11:27 a.m. UTC | #7 
Am Freitag, den 09.07.2021, 18:36 +0800 schrieb Zhu Zihao:
> Giovanni Biscuolo writes:
> 
> > Also, if you think it's useful in the future we could also add a
> > "emacs-telega-server-unstable" for testing bleading edge TDLib
> > features.
> > 
> > WDYT?
> > 
> > I'll give your patch series a try if I find some time this week.
> > 
> > Thanks! Gio'
> > 
> > 
> > [1] 
> > https://github.com/zevlg/telega.el/issues/297#issuecomment-868992025
> 
> I'm OK with it, maybe rename it to `emacs-telega-server-next`?
> 
> But I'm not sure Guix maintainer would accept it or not.
This probably depends on how often we'd have to bump TDLib to make this
meaningful.  "Major" release sounds like a 1.0 → 2.0 version bump,
which if "unstable" telega only depended on stuff introduced e.g. in
1.1 → 1.2 sounds a little too strict.  However, if unstable telega
means tailing arbitrary TDLib commits, I'd rather avoid doing that.

There's so far no precedent to worry about such things, though, so I'd
rather push the existing patch after some testing or an altered one if
problems are detected.  I haven't gotten to testing things myself
either, but given that we are one stable telega release behind, now
sounds like a good time to start doing that.  (Please pardon the
laziness on my part – I typically do lexical work before practical one
to allow myself to work on other patches and projects as well.)

Regards,
Leo
Leo Prikler July 9, 2021, 1:15 p.m. UTC | #8 
Am Freitag, den 09.07.2021, 13:27 +0200 schrieb Leo Prikler:
> Am Freitag, den 09.07.2021, 18:36 +0800 schrieb Zhu Zihao:
> > Giovanni Biscuolo writes:
> > 
> > > Also, if you think it's useful in the future we could also add a
> > > "emacs-telega-server-unstable" for testing bleading edge TDLib
> > > features.
> > > 
> > > WDYT?
> > > 
> > > I'll give your patch series a try if I find some time this week.
> > > 
> > > Thanks! Gio'
> > > 
> > > 
> > > [1] 
> > > https://github.com/zevlg/telega.el/issues/297#issuecomment-868992025
> > 
> > I'm OK with it, maybe rename it to `emacs-telega-server-next`?
> > 
> > But I'm not sure Guix maintainer would accept it or not.
> This probably depends on how often we'd have to bump TDLib to make
> this
> meaningful.  "Major" release sounds like a 1.0 → 2.0 version bump,
> which if "unstable" telega only depended on stuff introduced e.g. in
> 1.1 → 1.2 sounds a little too strict.  However, if unstable telega
> means tailing arbitrary TDLib commits, I'd rather avoid doing that.
> 
> There's so far no precedent to worry about such things, though, so
> I'd
> rather push the existing patch after some testing or an altered one
> if
> problems are detected.  I haven't gotten to testing things myself
> either, but given that we are one stable telega release behind, now
> sounds like a good time to start doing that.  (Please pardon the
> laziness on my part – I typically do lexical work before practical
> one
> to allow myself to work on other patches and projects as well.)
> 
> Regards,
> Leo
After some testing, I've decided to push the current patch set, albeit
with some changes.  I redid my old hardcoding of ffmpeg and rephrased
the commit messages to be more explicit.  I also followed up with a
bump to 0.7.025.

There might still be some discussion to be had about versioning, but
for now I will close this bug.

Regards,
Leo
diff mbox series

Patch

From aad569dfce7a3223192cd34be30ff083a6962e55 Mon Sep 17 00:00:00 2001
From: Zhu Zihao <all_but_last@163.com>
Date: Fri, 25 Jun 2021 13:29:30 +0800
Subject: [PATCH 3/3] gnu: emacs-telega-contrib: Update to 0.7.024.

* gnu/packages/emacs-xyz.scm(emacs-telega-contrib): Update to 0.7.024.

[arguments]<phases>: Back to root directory of build before phase
'install-license-files' to properly install licenses.
[propagated-inputs]: Add emacs-dashboard, emacs-transient.
---
 gnu/packages/emacs-xyz.scm | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 0535840293..5fa0a7d60e 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -26117,19 +26117,28 @@  service, and connect it with Emacs via inter-process communication.")
 for the Telegram messaging platform.")))
 
 (define-public emacs-telega-contrib
-  (package/inherit emacs-telega
+  (package
+    (inherit emacs-telega)
     (name "emacs-telega-contrib")
-    (build-system emacs-build-system)
     (arguments
      `(#:exclude '("telega-live-location.el")
        #:phases
        (modify-phases %standard-phases
-         (add-after 'unpack 'chdir
-           (lambda _ (chdir "contrib") #t)))))
+         (add-after 'unpack 'enter-subdirectory
+           (lambda _ (chdir "contrib") #t))
+         (add-before 'install-license-files 'leave-subdirectory
+           (lambda _ (chdir "..") #t)))))
+    (inputs '())
+    (native-inputs '())
     (propagated-inputs
-     `(("emacs-telega" ,emacs-telega)
-       ("emacs-alert" ,emacs-alert)
-       ("emacs-all-the-icons" ,emacs-all-the-icons)))))
+     `(("emacs-alert" ,emacs-alert)
+       ("emacs-all-the-icons" ,emacs-all-the-icons)
+       ("emacs-dashboard" ,emacs-dashboard)
+       ("emacs-telega" ,emacs-telega)
+       ("emacs-transient" ,emacs-transient)))
+    (synopsis "Contributed packages to Telega")
+    (description "Telega-contrib is a collection of third-party
+contributed packages to Telega.")))
 
 (define-public emacs-doom-modeline
   (package
-- 
2.32.0