x11-plugins/pidgin-telegram: New package #7777

ConiKost · 2018-04-02T21:10:19Z

x11-plugins/pidgin-telegram: New package

Closes: https://bugs.gentoo.org/529908

gentoo-repo-qa-bot · 2018-04-02T21:13:02Z

Pull Request assignment

Areas affected: ebuilds
Packages affected: x11-plugins/pidgin-telegram

x11-plugins/pidgin-telegram: @gentoo/proxy-maint (new package)

Bugs linked: 529908

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

monsieurp · 2018-04-07T16:46:10Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+S="${WORKDIR}/telegram-purple"
+
+src_prepare() {
+	# Remove '-Werror' to make it compile


local WERROR_FILES=( Makefile.mingw ... # the whole list of files ) local f for f in "${WERROR_FILES[@]}"; do sed -i -e 's/-Werror //' "${f}" || die done

That are 3 lines more :P Anyway, I've changed it :)

monsieurp · 2018-04-07T16:47:41Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+		emake DESTDIR="${D}" noicon_install
+
+		# Install docs
+		einstalldocs


Please get einstalldocs out of this condition. Documentation installation shouldn't depend on the icons USE flag.

I am not sure, if this is really wrong? As I need to call noicon_install for USE="-icon", I am also calling einstalldocs. But if I use anyway USE="icon", I run default, which also calls einstalldocs? Or I am wrong? Anyway, I have put it outside of this block.

Zlogene · 2018-04-28T07:55:53Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+		sed -i -e 's/-Werror //' "${werror_file}" || die
+	done
+
+	# Apply user patches


Please stop using trivial comkments, we pointed that many times

To be fair: This and the other eBuilds where created before you pointed out regarding the comments. So I hadn't updated it yet ;-) Fixed for this and the rest here.

I will look today through my other not yet updated eBuilds.

Zlogene · 2018-04-28T07:56:01Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+}
+
+src_configure() {
+	# Run 'configure'


Zlogene · 2018-04-28T07:56:11Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+}
+
+src_install() {
+	# Set and install docs


Zlogene · 2018-04-28T07:56:24Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+	local DOCS=( "AUTHORS" "CHANGELOG.md" "HACKING.md" "HACKING.BUILD.md" "README.md" )
+	einstalldocs
+
+	# Install icons depending on users choice


Zlogene · 2018-04-28T07:56:32Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+
+	# Install icons depending on users choice
+	if ! use icons; then
+		# Run 'make install'


Zlogene · 2018-04-28T07:56:40Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+		# Run 'make install'
+		emake DESTDIR="${D}" noicon_install
+	else
+		# Run 'default'


mgorny · 2018-06-03T07:41:38Z

x11-plugins/pidgin-telegram/metadata.xml

+		<name>Proxy Maintainers</name>
+	</maintainer>
+	<longdescription>
+		Telegram-purple is a libpurple protocol plugin that adds support for the Telegram messenger.


It doesn't add anything to DESCRIPTION.

Okok, added a better one.

mgorny · 2018-06-03T07:42:19Z

x11-plugins/pidgin-telegram/metadata.xml

+	</longdescription>
+	<use>
+		<flag name="gcrypt">Use dev-libs/libgcrypt instead of dev-libs/openssl.</flag>
+		<flag name="icons">Install nice icons.</flag>


Is there a use case for installing it without 'nice icons'? What happens then?

Okay, after looking through it, I tend to remove the no_icons variant and always install icons.
If not disabled, it installs default icons into /usr/share/pixmaps/, which, according to the readme, pidgin will use.

mgorny · 2018-06-03T07:42:51Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+
+DESCRIPTION="A libpurple protocol plugin that adds support for the Telegram messenger"
+HOMEPAGE="https://github.com/majn/telegram-purple"
+SRC_URI="https://github.com/majn/telegram-purple/releases/download/v${PV}/telegram-purple_${PV}.orig.tar.gz -> ${P}.tar.gz"


Why do you rename it?

Should I not? Ok, removed it.

mgorny · 2018-06-03T07:44:07Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+HOMEPAGE="https://github.com/majn/telegram-purple"
+SRC_URI="https://github.com/majn/telegram-purple/releases/download/v${PV}/telegram-purple_${PV}.orig.tar.gz -> ${P}.tar.gz"
+
+LICENSE="GPL-2"


Incorrect, please read the copyright notices in files.

Changed to GPL-2+

mgorny · 2018-06-03T07:44:22Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+	gcrypt? ( dev-libs/libgcrypt:0= )
+	!gcrypt? ( dev-libs/openssl:0= )
+	nls? ( sys-devel/gettext )
+	webp? ( media-libs/libwebp )


Thanks, added.

mgorny · 2018-06-03T07:45:13Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+	local werror_file
+	local WERROR_FILES=( "Makefile.mingw" "Makefile.tgl.mingw" "tgl/Makefile.in" "tgl/tl-parser/Makefile.in" )
+	for werror_file in "${WERROR_FILES[@]}"; do
+		sed -i -e 's/-Werror //' "${werror_file}" || die


sed takes multiple files, so don't loop here. Also, wouldn't it be easier to just find -name 'Makefile*'?

Sounds good, changed to find with sed.

mgorny · 2018-06-03T07:46:29Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+
+src_install() {
+	if ! use icons; then
+		emake DESTDIR="${D}" noicon_install


I dare say moving the conditional into $(usex icons ...) would be cleaner than conditionally using default.

Since I removed noicon_install, this won't be needed anymore.

mgorny · 2018-06-19T19:05:25Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+	default
+
+	# Remove '-Werror' to make it compile
+	find -name 'Makefile*' -exec sed -i -e 's/-Werror //'  {} \; || die


\; → +, sed accepts multiple paths.

mgorny · 2018-06-19T19:05:47Z

Since this is my only comment, I'll fix it locally and merge if it works fine.

mgorny

Found more issues. Please fix them all then.

mgorny · 2018-06-19T19:07:44Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+		$(use_enable gcrypt)
+		$(use_enable nls translation)
+		$(use_enable webp libwebp)
+		$(use_with zlib)


checking if zlib is wanted... yes configure: WARNING: Sorry, yes does not exist, checking usual places

Whoops, zlib is not optional. Fixed.

mgorny · 2018-06-19T19:08:07Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+	webp? ( media-libs/libwebp:= )
+	zlib? ( sys-libs/zlib:= )"
+
+DEPEND="${RDEPEND}"


Missing DEPEND:

checking for x86_64-pc-linux-gnu-pkg-config... (cached) /usr/bin/pkg-config checking pkg-config is at least version 0.9.0... yes

mgorny · 2018-06-20T07:28:39Z

x11-plugins/pidgin-telegram/pidgin-telegram-1.3.1.ebuild

+	nls? ( sys-devel/gettext )
+	webp? ( media-libs/libwebp:= )"
+
+DEPEND="dev-util/pkgconfig


Now please run repoman.

Closes: https://bugs.gentoo.org/529908 Package-Manager: Portage-2.3.40, Repoman-2.3.9

gentoo-repo-qa-bot · 2018-06-20T08:36:55Z

Pull request CI report

Report generated at: 2018-06-20 08:36 UTC
Newest commit scanned: 2015f35
Status: ✅ good

Issues already there before the PR (double-check them):
https://qa-reports.gentoo.org/output/gentoo-ci/857e02a/output.html#app-emulation/buildah

ConiKost force-pushed the pidgin-telegram branch 2 times, most recently from abaeaa5 to 33d03e1 Compare April 2, 2018 21:12

ConiKost force-pushed the pidgin-telegram branch from 33d03e1 to 2b22bd5 Compare April 3, 2018 08:44

monsieurp suggested changes Apr 7, 2018

View reviewed changes

ConiKost force-pushed the pidgin-telegram branch from 2b22bd5 to 1a6ce95 Compare April 7, 2018 17:32

ConiKost force-pushed the pidgin-telegram branch from 1a6ce95 to 6202ef8 Compare April 26, 2018 19:38

Zlogene suggested changes Apr 28, 2018

View reviewed changes

ConiKost force-pushed the pidgin-telegram branch 2 times, most recently from 058db5b to 03ec8d5 Compare April 28, 2018 09:37

ConiKost force-pushed the pidgin-telegram branch 4 times, most recently from da77b25 to e254fd7 Compare May 11, 2018 16:06

mgorny requested changes Jun 3, 2018

View reviewed changes

ConiKost force-pushed the pidgin-telegram branch from e254fd7 to a0a023c Compare June 19, 2018 18:27

mgorny reviewed Jun 19, 2018

View reviewed changes

mgorny requested changes Jun 19, 2018

View reviewed changes

ConiKost force-pushed the pidgin-telegram branch 2 times, most recently from aa14d17 to 6bbf4e5 Compare June 19, 2018 21:42

mgorny reviewed Jun 20, 2018

View reviewed changes

x11-plugins/pidgin-telegram: New package

2015f35

Closes: https://bugs.gentoo.org/529908 Package-Manager: Portage-2.3.40, Repoman-2.3.9

ConiKost force-pushed the pidgin-telegram branch from 6bbf4e5 to 2015f35 Compare June 20, 2018 07:34

mgorny approved these changes Jun 20, 2018

View reviewed changes

gentoo-bot closed this in a447990 Jun 20, 2018

x11-plugins/pidgin-telegram: New package #7777

x11-plugins/pidgin-telegram: New package #7777

Conversation

ConiKost commented Apr 2, 2018

gentoo-repo-qa-bot commented Apr 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ConiKost Apr 7, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ConiKost Apr 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgorny commented Jun 19, 2018

mgorny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gentoo-repo-qa-bot commented Jun 20, 2018

ConiKost Apr 7, 2018 •

edited

ConiKost Apr 28, 2018 •

edited