Bug 739088 - Review Request: arandr - Simple GTK+ XRandR GUI
Review Request: arandr - Simple GTK+ XRandR GUI
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2011-09-16 10:36 EDT by Maros Zatko
Modified: 2012-07-11 19:50 EDT (History)
12 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2012-07-11 19:50:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Maros Zatko 2011-09-16 10:36:14 EDT
Spec URL: http://v3.sk/~hexo/rpm/arandr.spec
SRPM URL: http://v3.sk/~hexo/rpm/arandr-0.1.4-1.fc15.src.rpm
Description: ARandR is designed to provide a simple visual front end for XRandR 1.2/1.3. Relative monitor positions are shown graphically and can be changed in a drag-and-drop way.
Comment 1 Veeti Paananen 2011-09-16 12:35:42 EDT
I can't sponsor you, but a few comments:

- You need to install the desktop file properly (see http://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files)

- You've accidentally put the year 2001 instead of 2011 into the changelog
Comment 2 Gregor Tätzner 2011-09-16 14:27:35 EDT
Hi Maros,
some additional feedback:

rpmlint RPMS/noarch/arandr-0.1.4-1.fc15.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

rpmlint SRPMS/arandr-0.1.4-1.fc15.src.rpm 
arandr.src:14: W: mixed-use-of-spaces-and-tabs (spaces: line 14, tab: line 2)
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

In the spec file you are mixing tabs and spaces. Don't do this. It's better to use spaces only.

the %doc entry is incomplete. You must add at least the license and readme, changelog, news and todo too, I think.
Comment 3 Maros Zatko 2011-09-17 10:55:38 EDT
Thank you for your feedback.

Updated files:
Spec URL: http://v3.sk/~hexo/rpm/arandr.spec
SRPM URL: http://v3.sk/~hexo/rpm/arandr-0.1.4-2.fc15.src.rpm
Comment 4 Gregor Tätzner 2011-09-17 16:04:25 EDT
-desktop file is properly validated
-changelog fixed
-tabs removed
-no rpmlint errors

I have only one complaint:
don't create the doc dir and copy doc files in the install section. the %doc macro handles this automatically.
Comment 5 Veeti Paananen 2011-09-17 16:18:49 EDT
You're mixing macro and variable styles for getting the build root (%{buildroot} and $RPM_BUILD_ROOT), which is against the guidelines [1]. Pick one and use it consistently. (IMO, the macro style is superior since macros are also used for other things like the directory paths. It's also shorter.)

[1] http://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS
Comment 6 Maros Zatko 2011-09-19 09:09:10 EDT
Hi, I've replaced variables with macros and fixed installation of doc files.

Please check:
Spec URL: http://v3.sk/~hexo/rpm/arandr.spec
SRPM URL: http://v3.sk/~hexo/rpm/arandr-0.1.4-3.fc15.src.rpm
Comment 7 Lubomir Rintel 2011-09-22 09:24:21 EDT
Taking for a formal review.
Comment 8 Lubomir Rintel 2011-09-22 09:35:30 EDT
* Package name correct
* Versioning correct
* RPMLint happy
* License is approved for fedora
- License tag correct
* Macros used consistently
- Spec file clean and legible
* American English used
* Filelist sane
* Requires sane
* Provides sane
* Builds fine in mock

1.) %{python_sitelib}/arandr-%{version}-py2.7.egg-info

This is wrong. It will only work for python 2.7, obviously.

2.) Fix your indentation:

Group:  Applications/System
License:  GPLv3
URL:    http://christian.amsuess.com/tools/arandr/
Source0:  http://christian.amsuess.com/tools/arandr/files/%{name}-%{version}.tar.gz
BuildRoot:  %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

3.) Licensing is bad

Files lack any copyright and licensing information at all: e.g.: screenlayout/xrandr.py screenlayout/meta.py. Needs to be fixed upstream.
Comment 9 chrysn 2011-10-01 10:57:35 EDT
hello redhat people, i am the upstream author.

i've added license header to all the python files in the development version; if there's anything else you need changed, now would be the time to let me know. if you need an official release in order to include the modified sources, i can push a or so with the updated file headers.

i'm not familiar with how the packages flow in the various redhat derivatives, but would like to include installation instructions to the arandr website once it gets shipped like there are now for debian/ubuntu and arch. could you provide an installation command, a list of distributions this works with, and kind of a package link?

thanks for packaging!
Comment 10 Maros Zatko 2011-10-03 09:28:17 EDT
great to have you here Chris!

it would be fine if you push bumped version with modified headers as a release.
i hope it is the last thing which is blocking this review.

once it is packaged and included in fedora it can be installed using yum at least in fedora 15:
# yum install arandr
Comment 11 chrysn 2011-10-09 10:14:58 EDT
there is now a release 0.1.5 that has every python file copyright-tagged. the gettext translations use their own format to indicate contributors, and are formatted in different ways depending on the tools the contributors use -- if you don't want to parse that, you could have a look at the copyright file i maintain for debian[1], it's the easiest way to know who touched what when.

[1] http://gitorious.org/arandr/arandr/blobs/debian/debian/copyright
Comment 12 chrysn 2012-06-02 10:50:30 EDT
short update: the latest version is now 0.1.6; can be obtained from
Comment 13 Maros Zatko 2012-06-04 06:16:57 EDT
thank you for updates.

I'll try to poke some folks and hopefully get this pushed to fedora during this week.
Comment 14 Maros Zatko 2012-07-10 10:34:46 EDT
so let's continue this saga:
SRPM: http://v3.sk/~hexo/rpm/arandr-0.1.6-1.fc17.src.rpm
SPEC: http://v3.sk/~hexo/rpm/arandr.spec
Comment 15 Lubomir Rintel 2012-07-10 10:39:15 EDT
Looks marvelous now, Maros!
May God bless you and your family.

Comment 16 Maros Zatko 2012-07-10 11:36:34 EDT
New Package SCM Request
Package Name: arandr
Short Description: Simple GTK+ XRandR GUI
Owners: mzatko
Branches: f17
Comment 17 Kevin Fenzi 2012-07-10 18:17:20 EDT
Git done (by process-git-requests).
Comment 18 Fedora Update System 2012-07-11 07:45:43 EDT
arandr-0.1.6-1.fc17 has been submitted as an update for Fedora 17.
Comment 19 Fedora Update System 2012-07-11 19:50:07 EDT
arandr-0.1.6-1.fc17 has been pushed to the Fedora 17 stable repository.

Note You need to log in before you can comment on or make changes to this bug.