Spec URL: http://ryanlerch.fedorapeople.org/packaging/corebird/corebird.spec SRPM URL: http://ryanlerch.fedorapeople.org/packaging/corebird/corebird-0.3-20131101git2e2962f.fc20.src.rpm Description: Native GTK Twitter client for the Linux desktop. Fedora Account System Username: ryanlerch
Hi, I'll review corebird and the required font sets. Thanks, Ankur
A 0.4 release was made 4 days ago. It's better to use this. You won't need to carry the git commit tag if you use this. I'll begin reviewing this version but will have to go over the updated srpm before I can approve the package https://github.com/baedert/corebird/releases/tag/0.4 Thanks, Warm regards, Ankur
(In reply to Ankur Sinha (FranciscoD) from comment #2) > A 0.4 release was made 4 days ago. *2 days ago. > It's better to use this. You won't need > to carry the git commit tag if you use this. I'll begin reviewing this > version but will have to go over the updated srpm before I can approve the > package > > https://github.com/baedert/corebird/releases/tag/0.4 > > Thanks, > Warm regards, > Ankur I'd better get some coffee.
Oh! It'll also be really great if you can write up an appdata file and include it in the package, and even better if you could also send it upstream.
(In reply to Ankur Sinha (FranciscoD) from comment #2) > A 0.4 release was made 4 days ago. It's better to use this. You won't need > to carry the git commit tag if you use this. I'll begin reviewing this > version but will have to go over the updated srpm before I can approve the > package > > https://github.com/baedert/corebird/releases/tag/0.4 > > Thanks, > Warm regards, > Ankur Thanks! i noticed this this morning too! here is the updated spec & SRPM to for the 0.4 release: http://ryanlerch.fedorapeople.org/packaging/corebird/corebird.spec http://ryanlerch.fedorapeople.org/packaging/corebird/corebird-0.4-0.fc20.src.rpm
Created attachment 820077 [details] Git diff patch. I'll go ahead and review the newer version. One nitpick, certainly not a blocker: Upstream forgot to bump the about-dialog version before tagging and did it in subsequent commits: * 003bee4 - (2 days ago) about dialog: 0.3 -> 0.4 — Timm Bäder * 42c0373 - (2 days ago) pkgbuild: Change _realver to 0.4 — Timm Bäder * 7b82914 - (2 days ago) readme: Remove ubuntu/fedora librsvg dependencies — Timm Bäder (tag: 0.4) Can you look to incorporate the two tiny changes (patch attached)? I think a simple sed command in %prep will do it. Or, you can use a tarball from this commit (this might be overkill). Users might get confused when they see a difference in version between the about dialog and rpm -q output. Detailed review coming up shortly. Thanks, Warm regards, Ankur
Thanks Ankur! I have updated the SRPM and the spec with this patch. http://ryanlerch.fedorapeople.org/packaging/corebird/2/
To me all scriptlets being used or mentioned in that page of wiki, I'd suggest that: Either from /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : to touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : Or from /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : to %{_bindir}/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :
(In reply to Christopher Meng from comment #8) > To me all scriptlets being used or mentioned in that page of wiki, I'd > suggest that: > > Either from > > /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : > > to > > touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : > > Or from > > /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : > > to > > %{_bindir}/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : Hi Christopher! specifically what wikipage are you referring to? regards, ryanlerch
(In reply to Ryan Lerch from comment #9) > Hi Christopher! > > specifically what wikipage are you referring to? > > regards, > ryanlerch I mean this: https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
(In reply to Christopher Meng from comment #10) > (In reply to Ryan Lerch from comment #9) > > Hi Christopher! > > > > specifically what wikipage are you referring to? > > > > regards, > > ryanlerch > > I mean this: > > https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache the example in that section of that wikipage suggests to use "/bin/touch" rather than just "touch". What is the rationale for suggesting this change?
FYI: With todays commit #9fea73447ebb86d82242e104d7804d78e221dbe8 corebird no longer uses fontawesome: https://github.com/baedert/corebird/commit/9fea73447ebb86d82242e104d7804d78e221dbe8
Hi, Upstream has released version 0.5 which does not depend on fontawesome and also contains a bunch of fixes and feature additions. I think the app is now in good enough state for end users, so we could continue the review and quickly get it into the Fedora repositories. Ryan, here's an updated spec for 0.5 (http://ankursinha.fedorapeople.org/corebird.spec) that you can refer Could you please submit an updated srpm that I could now formally review? Thanks, Warm regards, Ankur
Thanks ankur! the updated files are available here: http://ryanlerch.org/corebird/package/ cheers, ryanlerch
[+] OK [-] NA [?] Issue ** Mandatory review guidelines: ** [+] rpmlint output: [asinha@ankur-laptop SRPMS]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm ../SPECS/corebird.spec ./corebird-0.5-1.fc20.src.rpm corebird.src: W: invalid-url Source0: https://github.com/baedert/corebird/archive/corebird-0.5.tar.gz HTTP Error 404: Not Found corebird.x86_64: W: no-manual-page-for-binary corebird ../SPECS/corebird.spec: W: invalid-url Source0: https://github.com/baedert/corebird/archive/corebird-0.5.tar.gz HTTP Error 404: Not Found corebird.src: W: invalid-url Source0: https://github.com/baedert/corebird/archive/corebird-0.5.tar.gz HTTP Error 404: Not Found 4 packages and 1 specfiles checked; 0 errors, 4 warnings. [asinha@ankur-laptop SRPMS]$ [+] License is acceptable (...) [+] License field in spec is correct [+] License files included in package %docs if included in source package [+] License files installed when any subpackage combination is installed [+] Spec written in American English [+] Spec is legible [+] Sources match upstream unless altered to fix permissibility issues [+] Build succeeds on at least one primary arch [+] Build succeeds on all primary arches or has ExcludeArch + bugs filed [+] BuildRequires correct, justified where necessary [-] Locales handled with %find_lang, not %_datadir/locale/* [+] %post, %postun call ldconfig if package contains shared .so files [+] No bundled libs [-] Relocatability is justified [?] Package owns all directories it creates ^ Please also own the appdata directory. It's OK to co-own it since gnome-software isn't required for functioning of this package. [+] Package requires others for directories it uses but does not own [+] No duplication in %files unless necessary for license files [+] File permissions are sane [+] Package contains permissible code or content [-] Large docs go in -doc subpackage [+] %doc files not required at runtime [-] Static libs go in -static package/virtual Provides [-] Development files go in -devel package [-] -devel packages Require base with fully-versioned dependency, %_isa [+] No .la files [+] GUI app uses .desktop file, installs it with desktop-file-install [+] File list does not conflict with other packages' without justification [+] File names are valid UTF-8 ** Optional review guidelines: ** [-] Query upstream about including license files [-] Translations of description, summary [+] Builds in mock [+] Builds on all arches [+] Functions as described (e.g. no crashes) [+] Scriptlets are sane [-] Subpackages require base with fully-versioned dependency if sensible [-] .pc file subpackage placement is sensible [+] No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin [-] Include man pages if available Naming guidelines: [+] Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+ [+] Package names are sane [+] No naming conflicts [+] Spec file name matches base package name [+] Version is sane [+] Version does not contain ~ [+] Release is sane [+] %dist tag [+] Case used only when necessary [+] Renaming handled correctly Packaging guidelines: [+] Useful without external bits [+] No kmods [-] Pre-built binaries, libs removed in %prep [+] Sources contain only redistributable code or content [+] Spec format is sane [+] Package obeys FHS, except libexecdir, /run, /usr/target [+] No files in /bin, /sbin, /lib* on >= F17 [-] Programs run before FS mounting use /run instead of /var/run [-] Binaries in /bin, /sbin do not depend on files in /usr on < F17 [+] No files under /srv, /opt, /usr/local [+] Changelog in prescribed format [+] No Packager, Vendor, Copyright, PreReq tags [+] Summary does not end in a period [-] Correct BuildRoot tag on < EL6 [-] Correct %clean section on < EL6 [+] Requires correct, justified where necessary [asinha@ankur-laptop result]$ review-req-check == corebird-0.5-1.fc21.src.rpm == Provides: Requires: gtk3-devel >= 3.9 glib2-devel >= 2.38 rest-devel json-glib-devel libnotify-devel sqlite-devel libsoup-devel vala-devel cmake librsvg2-tools desktop-file-utils libgee-devel == corebird-0.5-1.fc21.x86_64.rpm == Provides: application() application(corebird.desktop) corebird = 0.5-1.fc21 corebird(x86-64) = 0.5-1.fc21 Requires: /bin/sh /bin/sh /bin/sh hicolor-icon-theme libatk-1.0.so.0()(64bit) libc.so.6()(64bit) libc.so.6(GLIBC_2.14)(64bit) libc.so.6(GLIBC_2.2.5)(64bit) libc.so.6(GLIBC_2.4)(64bit) libcairo-gobject.so.2()(64bit) libcairo.so.2()(64bit) libgdk-3.so.0()(64bit) libgdk_pixbuf-2.0.so.0()(64bit) libgee-0.8.so.2()(64bit) libgio-2.0.so.0()(64bit) libglib-2.0.so.0()(64bit) libgobject-2.0.so.0()(64bit) libgtk-3.so.0()(64bit) libjson-glib-1.0.so.0()(64bit) libnotify.so.4()(64bit) libpango-1.0.so.0()(64bit) libpangocairo-1.0.so.0()(64bit) librest-0.7.so.0()(64bit) libsoup-2.4.so.1()(64bit) libsqlite3.so.0()(64bit) libxml2.so.2()(64bit) rtld(GNU_HASH) == corebird-debuginfo-0.5-1.fc21.x86_64.rpm == Provides: corebird-debuginfo = 0.5-1.fc21 corebird-debuginfo(x86-64) = 0.5-1.fc21 Requires: [+] Summary, description do not use trademarks incorrectly [+] All relevant documentation is packaged, appropriately marked with %doc [+] Doc files do not drag in extra dependencies (e.g. due to +x) [+] Code compilable with gcc is compiled with gcc [+] Build honors applicable compiler flags or justifies otherwise [-] PIE used for long-running/root daemons, setuid/filecap programs [+] Useful -debuginfo package or disabled and justified [-] Package with .pc files Requires pkgconfig on < EL6 [+] No static executables [+] Rpath absent or only used for internal libs [-] Config files marked with %config(noreplace) or justified %config [+] No config files under /usr [-] Third party package manager configs acceptable, in %_docdir [+] .desktop files are sane [+] Spec uses macros consistently [+] Spec uses macros instead of hard-coded names where appropriate [+] Spec uses macros for executables only when configurability is needed [-] %makeinstall used only when alternatives don't work [-] Macros in Summary, description are expandable at srpm build time [+] Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir [+] No software collections (scl) [-] Macro files named /etc/rpm/macros.%name [+] Build uses only python/perl/shell+coreutils/lua/BuildRequired langs [-] %global, not %define [-] Package translating with gettext BuildRequires it [-] Package translating with Linguist BuildRequires qt-devel [+] File ops preserve timestamps [+] Parallel make [+] No Requires(pre,post) notation [-] User, group creation handled correctly (See Packaging:UsersAndGroups) [-] Web apps go in /usr/share/%name, not /var/www [-] Conflicts are justified [+] One project per package [+] No bundled fonts [-] Patches have appropriate commentary [-] Available test suites executed in %check [-] tmpfiles.d used for /run, /run/lock on >= F15 PACKAGE IS *** APPROVED *** Tiny issues you should correct before importing: - No need to remove fonts from the ./data directory. They aren't included in the tar any more. - Please co-own the appdata directory Thanks, Warm regards, Ankur
(In reply to Ankur Sinha (FranciscoD) from comment #15) > > PACKAGE IS *** APPROVED *** > > Tiny issues you should correct before importing: > > - No need to remove fonts from the ./data directory. They aren't included in > the tar any more. Okay, will remove the following lines from the spec: """ # in separate package rm -rf %{buildroot}%{_datadir}/fonts """ > - Please co-own the appdata directory Not 100% sure what to do to achieve this. Also, I originally forgot to make this block the NEED_SPONSOR bug, as this is my first package. cheers, ryanlerch
I am sponsoring ryanlerch into the packager group.
I made the following changes to the spec to solve the last 2 final issues: """ @@ -1,6 +1,6 @@ Name: corebird Version: 0.5 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Native GTK Twitter client License: GPLv3+ URL: http://corebird.baedert.org/ @@ -36,9 +36,6 @@ desktop-file-validate %{buildroot}/%{_datadir}/applications/%{name}.desktop -# in separate package -rm -rf %{buildroot}%{_datadir}/fonts - %post /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || : @@ -60,9 +57,13 @@ %{_datadir}/%{name}/ %{_datadir}/glib-2.0/schemas/org.baedert.%{name}.gschema.xml %{_datadir}/icons/hicolor/*/apps/%{name}.png -%{_datadir}/appdata/%{name}.appdata.xml +%{_datadir}/appdata/ %changelog +* Tue Dec 03 2013 Ryan Lerch <ryanlerch> 0.5-2 +- Removed line from the spec that removed the old font files +- Changed the spec so the appdata dir is co-owned + * Fri Nov 29 2013 Ankur Sinha <ankursinha AT fedoraproject DOT org> 0.5-1 - Update to 0.5 """
New Package SCM Request ======================= Package Name: corebird Short Description: Native GTK Twitter client Owners: ryanlerch Branches: f20 InitialCC: threebean
threebean is not a vaid FAS account.
Doh. I should've caught that. It should have been 'ralph'. Ryan, can you re-do the scm admin request changing the initialcc from threebean to ralph?
New Package SCM Request ======================= Package Name: corebird Short Description: Native GTK Twitter client Owners: ryanlerch Branches: f20 InitialCC: ralph
Git done (by process-git-requests).
corebird-0.5-2.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/corebird-0.5-2.fc20
corebird-0.5-2.fc20 has been pushed to the Fedora 20 stable repository.