[Bug 1291060] Review Request: purple-telegram - adds support for Telegram to Pidgin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1291060



--- Comment #1 from Michael Schwendt <bugs.michael@xxxxxxx> ---
A brief look:

Consider pointing the fedora-review tool at this ticket. It will download the
latest package from the "Spec URL:" and "SRPM URL:" lines, perform a local Mock
build and run lots of checks on the packages, which are of interest to all
packagers not only during official package review.


> BuildRequires:	gettext,libgcrypt-devel,pkgconfig(zlib),pkgconfig(purple),pkgconfig(libwebp)

Note that while this line of BuildRequires works, it is considered much more
readable and maintainable to split individual build requirements to separate
lines, especially if there were many more BR, and also if you ever needed to
delete one, doing that with individual lines is more convenient:

  BuildRequires: gettext
  BuildRequires: libgcrypt-devel
  BuildRequires: pkgconfig(zlib)
  BuildRequires: pkgconfig(purple)
  BuildRequires: pkgconfig(libwebp)


> %find_lang telegram-purple

You run %find_lang here without using its output file "telegram-purple.lang".
You need to pass that file to the %files section:

  %files -f telegram-purple.lang

https://fedoraproject.org/wiki/Packaging:Guidelines#Why_do_we_need_to_use_.25find_lang.3F


> %{_libdir}/purple-2/telegram-purple.so

That directory is provided by libpurple, but then this plugin needs to do
something about ownership of the "pidgin" icon directories further below:


> %config %{_sysconfdir}/telegram-purple/*

Directory  /etc/telegram-purple  is not included yet:
https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership


> #Icons
> %{_datadir}/pixmaps/pidgin/protocols/16/telegram.png

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership


> #Translations
> %{_datadir}/locale/de/LC_MESSAGES/telegram-purple.mo

Due to not using %find_lang correctly.


> #AppStream metadata
> %{_datadir}/appdata/telegram-purple.metainfo.xml

https://fedoraproject.org/wiki/Packaging:Guidelines#AppData_files
https://fedoraproject.org/wiki/Packaging:AppData

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]