Add hexchat snap #171

Merged
merged 36 commits into from Jul 15, 2016

Conversation

Projects
None yet
4 participants
Contributor

hikiko commented Jul 13, 2016

added hexchat to snappy playpen


This change is Reviewable

Eleni Maria Stea added some commits Jul 13, 2016

Collaborator

dholbach commented Jul 13, 2016

hexchat/README.md, line 1 [r1] (raw file):

# $PROJECT hexchat

Maybe "hextchat snap" instead?


Comments from Reviewable

@hikiko hikiko changed the title from added hexchat to snappy playpen to Add hexchat snap Jul 13, 2016

Collaborator

dholbach commented Jul 13, 2016

hexchat/snapcraft.yaml, line 4 [r1] (raw file):

version: 0.1
summary: IRC client for X based on X-Chat 2
description: HexChat is a graphical IRC client with a GTK+ GUI. Features include Python and Perl scripting support, a plugin API, multiple server/channel windows, spell checking, multiple authentication methods including SASL, and customizable notifications. For more information on IRC, see http://irchelp.org/.

You can use something like this to wrap the lines at 80 chars:

description: |
  HexChat is a graphical IRC client with a GTK+ GUI. Features include Python    
  and Perl scripting support, a plugin API, multiple server/channel windows,    
  spell checking, multiple authentication methods including SASL, and           
  customizable notifications. For more information on IRC, see                  
  http://irchelp.org/.

---


*Comments from [Reviewable](https://reviewable.io:443/reviews/ubuntu/snappy-playpen/171)*
<!-- Sent from Reviewable.io -->
Collaborator

dholbach commented Jul 13, 2016

hexchat/snapcraft.yaml, line 13 [r1] (raw file):

        build-packages:
            - hexchat-common

Is this really required for building?


Comments from Reviewable

Collaborator

dholbach commented Jul 13, 2016

hexchat/snapcraft.yaml, line 14 [r1] (raw file):

        build-packages:
            - hexchat-common
            - libc6

Are you sure not the relevant lib*-dev packages are needed?


Comments from Reviewable

Collaborator

dholbach commented Jul 13, 2016

snap-template/README.md, line 0 [r1] (raw file):
Why is this deleted?


Comments from Reviewable

Collaborator

dholbach commented Jul 13, 2016

snap-template/snapcraft.yaml, line 0 [r1] (raw file):
Why is this deleted?


Comments from Reviewable

Collaborator

dholbach commented Jul 13, 2016

hexchat/snapcraft.yaml, line 14 [r1] (raw file):

Previously, dholbach (Daniel Holbach) wrote…

Are you sure not the relevant lib*-dev packages are needed?

https://travis-ci.org/ubuntu/snappy-playpen/builds/144407862 fails too. It will at least need `intltool` to pass this particular issue. Maybe take a look `apt-cache showsrc hexchat` to get an idea what it requires in Ubuntu to build?

Comments from Reviewable

Eleni Maria Stea added some commits Jul 13, 2016

Collaborator

dholbach commented Jul 13, 2016

hexchat/snapcraft.yaml, line 14 [r1] (raw file):

Previously, dholbach (Daniel Holbach) wrote…

https://travis-ci.org/ubuntu/snappy-playpen/builds/144407862 fails too. It will at least need intltool to pass this particular issue. Maybe take a look apt-cache showsrc hexchat to get an idea what it requires in Ubuntu to build?

Maybe add `imagemagick, intltool, libcanberra-dev, libdbus-glib-1-dev, libglib2.0-dev, libgtk2.0-dev, libnotify-dev, libpci-dev, libperl-dev, libproxy-dev, libssl-dev, libtool, python-dev`?

Comments from Reviewable

Eleni Maria Stea added some commits Jul 13, 2016

Contributor

hikiko commented Jul 13, 2016

Thanks for the review :) I've fixed several things, I hope it looks better now.

Collaborator

dholbach commented Jul 13, 2016

hexchat/snapcraft.yaml, line 17 [r4] (raw file):

            - liblua5.3-dev
            - libluajit-5.1-dev
            - debhelper

debhelper won't be needed.


Comments from Reviewable

Collaborator

dholbach commented Jul 13, 2016

hexchat/snapcraft.yaml, line 4 [r1] (raw file):

Previously, dholbach (Daniel Holbach) wrote…

You can use something like this to wrap the lines at 80 chars:

description: |
  HexChat is a graphical IRC client with a GTK+ GUI. Features include Python    
  and Perl scripting support, a plugin API, multiple server/channel windows,    
  spell checking, multiple authentication methods including SASL, and           
  customizable notifications. For more information on IRC, see                  
  http://irchelp.org/.
</details>
Wrapping would look nicer, but not a big issue.

Comments from Reviewable

Collaborator

dholbach commented Jul 13, 2016

snap-template/README.md, line [r1] (raw file):

Previously, dholbach (Daniel Holbach) wrote…

Why is this deleted?

It'd be nice if this could be re-added.

Comments from Reviewable

Collaborator

dholbach commented Jul 13, 2016

snap-template/snapcraft.yaml, line [r1] (raw file):

Previously, dholbach (Daniel Holbach) wrote…

Why is this deleted?

It'd be nice if this could be re-added.

Comments from Reviewable

Eleni Maria Stea added some commits Jul 14, 2016

hexchat/snapcraft.yaml
@@ -0,0 +1,37 @@
+name: hexchat
+version: 0.1
@tsimonq2

tsimonq2 Jul 14, 2016

Contributor

version: "0.1"

hexchat/snapcraft.yaml
@@ -0,0 +1,37 @@
+name: hexchat
+version: 0.1
+summary: IRC client for X based on X-Chat 2
@tsimonq2

tsimonq2 Jul 14, 2016

Contributor

this doesn't make sense, or am I wrong?

@hikiko

hikiko Jul 14, 2016

Contributor

That's the short description that is returned by apt-cache search hexchat.

hexchat/snapcraft.yaml
+version: 0.1
+summary: IRC client for X based on X-Chat 2
+description: |
+ HexChat is a graphical IRC client with a GTK+ GUI. Features include Python and Perl scripting support, a plugin API, multiple server/channel windows, spell checking, multiple authentication methods including SASL, and customizable notifications. For more information on IRC, see http://irchelp.org/.
@tsimonq2

tsimonq2 Jul 14, 2016

Contributor

Could you wrap at 80 chars please?

hexchat/snapcraft.yaml
+ hexchat:
+ plugin: autotools
+ source: git://github.com/hikiko/hexchat.git
+
@tsimonq2

tsimonq2 Jul 14, 2016

Contributor

this empty line isn't needed

Contributor

tsimonq2 commented Jul 14, 2016

Could you change the indent from four (4) spaces to two (2) spaces please?

Collaborator

dplanella commented Jul 14, 2016

Hm... the snap does not work for me:

$ hexchat 

(process:19463): Gtk-WARNING **: Locale not supported by C library.
    Using the fallback 'C' locale.
Gtk-Message: Failed to load module "overlay-scrollbar"
Gtk-Message: Failed to load module "gail"
Gtk-Message: Failed to load module "atk-bridge"
Gtk-Message: Failed to load module "unity-gtk-module"

(hexchat:19463): Gtk-WARNING **: cannot open display: :0

Have you tried using the desktop launcher? I can't work out how you are setting up the environment with just calling the binary and without a launcher.

Eleni Maria Stea and others added some commits Jul 14, 2016

Collaborator

dplanella commented Jul 14, 2016

I've submitted a PR with some small changes, as once I got the app to launch, it wouldn't connect to the IRC servers: https://github.com/hikiko/snappy-playpen/pull/1

Also, it would be good to add an icon and a desktop file under the setup/gui directory (you can have a look at the other examples in the playpen to see how it's done).

Eleni Maria Stea added some commits Jul 14, 2016

Collaborator

dplanella commented Jul 14, 2016

Thanks for adding the desktop launcher. I'm not sure if you saw the PR against your branch mentioned above, but at least on my case, I had to add the network-bind plug for hexchat to connect to IRC servers. It might be worth double-checking that.

Collaborator

dholbach commented Jul 14, 2016

hexchat/snapcraft.yaml, line 3 [r5] (raw file):

Previously, hikiko (Eleni Maria S.) wrote…

That's the short description that is returned by apt-cache search hexchat.

I think it's fine.

Comments from Reviewable

Collaborator

dholbach commented Jul 14, 2016

hexchat/snapcraft.yaml, line 2 [r5] (raw file):

Previously, tsimonq2 (Simon Quigley) wrote…

version: "0.1"

Not a blocker.

Comments from Reviewable

Eleni Maria Stea and others added some commits Jul 14, 2016

Merge pull request #2 from hikiko/dplanella-hexchat
Dplanella's hexchat fixes
hexchat/snapcraft.yaml
+ hexchat:
+ plugin: autotools
+ source: git://github.com/hikiko/hexchat.git
+
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

why the extra line here?

hexchat/snapcraft.yaml
+ snap:
+ - -lib/pkgconfig
+ after: [desktop/gtk2]
+apps:
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

from what I've seen, the apps: tag usually goes above the parts: tag

hexchat/snapcraft.yaml
+apps:
+ hexchat:
+ command: desktop-launch $SNAP/bin/hexchat
+ plugs: [network, x11, network-bind, unity7]
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

something picky, could you put network and network-bind together? So like this:

plugs: [network, network-bind, x11, unity7]

Eleni Maria Stea added some commits Jul 15, 2016

Contributor

tsimonq2 commented Jul 15, 2016

@hikiko I think that unless you specify the snap in the snapcraft.yaml file, it should be HexChat, not Hexchat, hexchat, or HEXCHAT. That's the branding on their website.

hexchat/README.md
+
+Working features: The snap can successfully install hexchat
+
+Known issues: We use a fork of hexchat (https://github.com/hikiko/hexchat) until
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

I would word it like:

Known issues: This snap currently uses a fork of hexchat (https://github.com/hikiko/hexchat) until

hexchat/README.md
+Working features: The snap can successfully install hexchat
+
+Known issues: We use a fork of hexchat (https://github.com/hikiko/hexchat) until
+a compile error in the master hexchat branch is fixed (there has been a merge
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

pull request, not merge proposal. :)

hexchat/README.md
+a compile error in the master hexchat branch is fixed (there has been a merge
+proposal).
+
+TODO: replace the fork branch with the master branch as soon as the compile
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

forked repository

hexchat/snapcraft.yaml
+parts:
+ hexchat:
+ plugin: autotools
+ source: git://github.com/hikiko/hexchat.git
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

something picky that I usually do, change git:// to https:// because the protocol used in git:// isn't secure

Eleni Maria Stea added some commits Jul 15, 2016

- fixes in README
- replaced git with https in source
hexchat/README.md
@@ -0,0 +1,14 @@
+# HEXCHAT snap
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

@hikiko this still needs to be fixed

hexchat/README.md
+
+Known issues: This snap currently uses a fork of HexChat
+(https://github.com/hikiko/hexchat) until a compile error in the master hexchat
+branch is fixed (there has been a pull request).
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

(there has been a pull request submitted) or (I have submitted a pull request)

hexchat/README.md
+
+## Current state
+
+Working features: The snap can successfully install hexchat
@tsimonq2

tsimonq2 Jul 15, 2016

Contributor

still needs to be fixed

Eleni Maria Stea added some commits Jul 15, 2016

Collaborator

dholbach commented Jul 15, 2016

hexchat/snapcraft.yaml, line 36 [r13] (raw file):

Previously, tsimonq2 (Simon Quigley) wrote…

from what I've seen, the apps: tag usually goes above the parts: tag

It doesn't matter.

Comments from Reviewable

Collaborator

dholbach commented Jul 15, 2016

:lgtm:


Comments from Reviewable

Collaborator

dholbach commented Jul 15, 2016

Reviewed 4 of 4 files at r1, 2 of 2 files at r2, 1 of 1 files at r4, 2 of 3 files at r5, 2 of 2 files at r7, 1 of 1 files at r8, 1 of 1 files at r12, 2 of 3 files at r13, 2 of 2 files at r17.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@dholbach dholbach merged commit f2f2923 into Ubuntu:master Jul 15, 2016

2 checks passed

code-review/reviewable 4 files reviewed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment