From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Hashe Subject: Re: [PATCH] gnu: Add rhythmbox. Date: Thu, 18 Jun 2015 21:09:06 -0500 Message-ID: References: <1434604057-17996-1-git-send-email-david.hashe@dhashe.com> <87bngdi0z0.fsf@elephly.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=e89a8ff1c510b029f50518d56858 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:45990) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5ljy-0005fc-0h for guix-devel@gnu.org; Thu, 18 Jun 2015 22:09:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5ljw-0005EQ-6w for guix-devel@gnu.org; Thu, 18 Jun 2015 22:09:09 -0400 Received: from mail-pd0-x22e.google.com ([2607:f8b0:400e:c02::22e]:36860) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5ljv-0005ED-Qr for guix-devel@gnu.org; Thu, 18 Jun 2015 22:09:08 -0400 Received: by pdjm12 with SMTP id m12so79185930pdj.3 for ; Thu, 18 Jun 2015 19:09:07 -0700 (PDT) In-Reply-To: <87bngdi0z0.fsf@elephly.net> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Ricardo Wurmus Cc: guix-devel@gnu.org --e89a8ff1c510b029f50518d56858 Content-Type: multipart/alternative; boundary=e89a8ff1c510b029f10518d56856 --e89a8ff1c510b029f10518d56856 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Thanks for the review. On Thu, Jun 18, 2015 at 8:45 AM, Ricardo Wurmus wrote: > > David Hashe writes: > > > * gnu/packages/gnome.scm (rhythmbox): New variable. > > [...] > > > +(define-public rhythmbox > > + (package > > + (name "rhythmbox") > > + (version "3.2.1") > > + (source (origin > > + (method url-fetch) > > + (uri (string-append "mirror://gnome/sources/rhythmbox/3.2/= " > > Can you use (version-major+minor version) instead of =E2=80=9C3.2=E2=80= =9D here? > > I also replaced each instance of "rhythmbox" with name. > > + "rhythmbox-" version ".tar.xz")) > > + (sha256 > > + (base32 > > + "0f3radhlji7rxl760yl2vm49fvfslympxrpm8497acbmbd7wlhxz"))= )) > > + (build-system glib-or-gtk-build-system) > > + (native-inputs > > + `(("intltool" ,intltool) > > + ("glib" ,glib "bin") > > + ("gobject-introspection" ,gobject-introspection) > > + ("pkg-config" ,pkg-config))) > > The indentation of (native-inputs ...) is wrong. > > + (inputs > > + `(("json-glib" ,json-glib) > > [...] > > > + ("brasero" ,brasero))) > > Is Brasero an optional input? It=E2=80=99s a CD burning application, whi= ch > seems unrelated to a music player. Will Totem work even if Brasero is > not available at build time? Or does it integrate more deeply with > Brasero? > > It's an optional input. Rhythmbox uses it (specifically libbrasero-media) to allow burning playlists straight to CD. Your comment also made me realize that I had the home-page wrong; I've fixed that. > > + (description "Rhythmbox is a music playing application for GNOME. I= t > supports > > +playlists, song ratings, and any codecs installed through > > gstreamer.") > > Please use two spaces at the end of a sentence. > > ~~ Ricardo > > Updated patch attached. David --e89a8ff1c510b029f10518d56856 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Thanks for the review.

On Thu, Jun 18, 2015 at= 8:45 AM, Ricardo Wurmus <rekado@elephly.net> wrote:

David Hashe <david.hashe@dhash= e.com> writes:

> * gnu/packages/gnome.scm (rhythmbox): New variable.

[...]

> +(define-public rhythmbox
> + (package
> +=C2=A0 =C2=A0(name "rhythmbox")
> +=C2=A0 =C2=A0(version "3.2.1")
> +=C2=A0 =C2=A0(source (origin
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (method url-fetch)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (uri (string-append "m= irror://gnome/sources/rhythmbox/3.2/"

Can you use (version-major+minor version) instead of =E2=80=9C3.2=E2= =80=9D here?


I also replace= d each instance of "rhythmbox" with name.
=C2=A0
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "rhythmbox-" version &= quot;.tar.xz"))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (sha256
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(base32
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "0f3radhlji7rxl= 760yl2vm49fvfslympxrpm8497acbmbd7wlhxz"))))
> +=C2=A0 =C2=A0(build-system glib-or-gtk-build-system)
> +=C2=A0 (native-inputs
> +=C2=A0 =C2=A0 `(("intltool" ,intltool)
> +=C2=A0 =C2=A0 =C2=A0 ("glib" ,glib "bin")
> +=C2=A0 =C2=A0 =C2=A0 ("gobject-introspection" ,gobject-intr= ospection)
> +=C2=A0 =C2=A0 =C2=A0 ("pkg-config" ,pkg-config)))

The indentation of (native-inputs ...) is wrong.
=C2=A0
<= span class=3D""> > +=C2=A0 =C2=A0(inputs
> +=C2=A0 =C2=A0 `(("json-glib" ,json-glib)

[...]

> +=C2=A0 =C2=A0 =C2=A0 ("brasero" ,brasero)))

Is Brasero an optional input?=C2=A0 It=E2=80=99s a CD burning application, = which
seems unrelated to a music player.=C2=A0 Will Totem work even if Brasero is=
not available at build time?=C2=A0 Or does it integrate more deeply with Brasero?


It's an op= tional input. Rhythmbox uses it (specifically libbrasero-media) to allow bu= rning playlists straight to CD.

Your comment also made me= realize that I had the home-page wrong; I've fixed that.
=C2=A0
> +=C2=A0 =C2=A0(description "Rhythmbox is a music playing applicat= ion for GNOME. It supports
> +playlists, song ratings, and any codecs installed through
> gstreamer.")

Please use two spaces at the end of a sentence.

~~ Ricardo


Updat= ed patch attached.

David
<= /div>
--e89a8ff1c510b029f10518d56856-- --e89a8ff1c510b029f50518d56858 Content-Type: text/x-patch; charset=US-ASCII; name="0001-gnu-Add-rhythmbox.patch" Content-Disposition: attachment; filename="0001-gnu-Add-rhythmbox.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_ib2z6oni0 RnJvbSBlZGEyZThlNzVjMGYyMWRhZTVlYmZlYzFhMjM3NjgzMGQ4MTA0ZDcxIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBEYXZpZCBIYXNoZSA8ZGF2aWQuaGFzaGVAZGhhc2hlLmNvbT4K RGF0ZTogV2VkLCAxNyBKdW4gMjAxNSAyMzo1OToxMSAtMDUwMApTdWJqZWN0OiBbUEFUQ0hdIGdu dTogQWRkIHJoeXRobWJveC4KCiogZ251L3BhY2thZ2VzL2dub21lLnNjbSAocmh5dGhtYm94KTog TmV3IHZhcmlhYmxlLgotLS0KIGdudS9wYWNrYWdlcy9nbm9tZS5zY20gfCA0OSArKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrCiAxIGZpbGUgY2hhbmdlZCwg NDkgaW5zZXJ0aW9ucygrKQoKZGlmZiAtLWdpdCBhL2dudS9wYWNrYWdlcy9nbm9tZS5zY20gYi9n bnUvcGFja2FnZXMvZ25vbWUuc2NtCmluZGV4IDRhZjFkMTMuLmVlMmJmNDcgMTAwNjQ0Ci0tLSBh L2dudS9wYWNrYWdlcy9nbm9tZS5zY20KKysrIGIvZ251L3BhY2thZ2VzL2dub21lLnNjbQpAQCAt MjQ0NCwzICsyNDQ0LDUyIEBAIHdoaWNoIGFyZSBlYXN5IHRvIHBsYXkgd2l0aCB0aGUgYWlkIG9m IGEgbW91c2UuIikKIG5hdGl2ZWx5IHdpdGggR1RLLURvYyAodGhlIEFQSSByZWZlcmVuY2Ugc3lz dGVtIGRldmVsb3BlZCBmb3IgR1RLKyBhbmQgdXNlZAogdGhyb3VnaG91dCBHTk9NRSBmb3IgQVBJ IGRvY3VtZW50YXRpb24pLiIpCiAgICAgKGxpY2Vuc2UgbGljZW5zZTpncGwyKykpKQorCisoZGVm aW5lLXB1YmxpYyByaHl0aG1ib3gKKyAocGFja2FnZQorICAgKG5hbWUgInJoeXRobWJveCIpCisg ICAodmVyc2lvbiAiMy4yLjEiKQorICAgKHNvdXJjZSAob3JpZ2luCisgICAgICAgICAgICAobWV0 aG9kIHVybC1mZXRjaCkKKyAgICAgICAgICAgICh1cmkgKHN0cmluZy1hcHBlbmQgIm1pcnJvcjov L2dub21lL3NvdXJjZXMvIiBuYW1lICIvIgorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAodmVyc2lvbi1tYWpvcittaW5vciB2ZXJzaW9uKSAiLyIKKyAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgbmFtZSAiLSIgdmVyc2lvbiAiLnRhci54eiIpKQorICAgICAgICAgICAgKHNo YTI1NgorICAgICAgICAgICAgIChiYXNlMzIKKyAgICAgICAgICAgICAgIjBmM3JhZGhsamk3cnhs NzYweWwydm00OWZ2ZnNseW1weHJwbTg0OTdhY2JtYmQ3d2xoeHoiKSkpKQorICAgKGJ1aWxkLXN5 c3RlbSBnbGliLW9yLWd0ay1idWlsZC1zeXN0ZW0pCisgICAobmF0aXZlLWlucHV0cworICAgIGAo KCJpbnRsdG9vbCIgLGludGx0b29sKQorICAgICAgKCJnbGliIiAsZ2xpYiAiYmluIikKKyAgICAg ICgiZ29iamVjdC1pbnRyb3NwZWN0aW9uIiAsZ29iamVjdC1pbnRyb3NwZWN0aW9uKQorICAgICAg KCJwa2ctY29uZmlnIiAscGtnLWNvbmZpZykpKQorICAgKGlucHV0cworICAgIGAoKCJqc29uLWds aWIiICxqc29uLWdsaWIpCisgICAgICAoImxpYnBlYXMiICxsaWJwZWFzKQorICAgICAgKCJ0ZGIi ICx0ZGIpCisgICAgICAoInRvdGVtLXBsLXBhcnNlciIgLHRvdGVtLXBsLXBhcnNlcikKKyAgICAg ICgid2Via2l0Z3RrIiAsd2Via2l0Z3RrKQorICAgICAgKCJnbm9tZS1kZXNrdG9wIiAsZ25vbWUt ZGVza3RvcCkKKyAgICAgICgicHl0aG9uIiAscHl0aG9uKQorICAgICAgKCJnbWltZSIgLGdtaW1l KQorICAgICAgKCJuZXR0bGUiICxuZXR0bGUpCisgICAgICAoIml0c3Rvb2wiICxpdHN0b29sKQor ICAgICAgKCJhZHdhaXRhLWljb24tdGhlbWUiICxhZHdhaXRhLWljb24tdGhlbWUpCisgICAgICAo ImdzdC1wbHVnaW5zLWJhc2UiICxnc3QtcGx1Z2lucy1iYXNlKQorICAgICAgKCJnc3QtcGx1Z2lu cy1nb29kIiAsZ3N0LXBsdWdpbnMtZ29vZCkKKyAgICAgICgiZ3N0cmVhbWVyIiAsZ3N0cmVhbWVy KQorICAgICAgKCJndWRldiIgLGV1ZGV2KQorICAgICAgKCJsaWJtdHAiICxsaWJtdHApCisgICAg ICAoImxpYnNlY3JldCIgLGxpYnNlY3JldCkKKyAgICAgICgibGlibm90aWZ5IiAsbGlibm90aWZ5 KQorICAgICAgKCJnc2V0dGluZ3MtZGVza3RvcC1zY2hlbWFzIiAsZ3NldHRpbmdzLWRlc2t0b3At c2NoZW1hcykKKyAgICAgICgiYXRrIiAsYXRrKQorICAgICAgKCJwYW5nbyIgLHBhbmdvKQorICAg ICAgKCJkZXNrdG9wLWZpbGUtdXRpbHMiICxkZXNrdG9wLWZpbGUtdXRpbHMpCisgICAgICAoImJy YXNlcm8iICxicmFzZXJvKSkpCisgICAoaG9tZS1wYWdlICJodHRwczovL3dpa2kuZ25vbWUub3Jn L0FwcHMvUmh5dGhtYm94IikKKyAgIChzeW5vcHNpcyAiTXVzaWMgcGxheWVyIGZvciBHTk9NRSIp CisgICAoZGVzY3JpcHRpb24gIlJoeXRobWJveCBpcyBhIG11c2ljIHBsYXlpbmcgYXBwbGljYXRp b24gZm9yIEdOT01FLiAgSXQKK3N1cHBvcnRzIHBsYXlsaXN0cywgc29uZyByYXRpbmdzLCBhbmQg YW55IGNvZGVjcyBpbnN0YWxsZWQgdGhyb3VnaCBnc3RyZWFtZXIuIikKKyAgIChsaWNlbnNlIGxp Y2Vuc2U6Z3BsMispKSkKKwotLSAKMS45LjEKCg== --e89a8ff1c510b029f50518d56858--