[Bug 1184040] Review Request: blivet-gui - Tool for data storage configuration

[bugzilla at redhat.com]

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

Jaroslav Škarvada <jskarvad at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-review?



--- Comment #3 from Jaroslav Škarvada <jskarvad at redhat.com> ---
I will sponsor you after we finish the review.

Issues found:

- SPEC file provided in comment 0 is different than SPEC file in SRPM, diff:
@@ -33,5 +33,4 @@

 %install
-rm -rf %{buildroot}
 make DESTDIR=%{buildroot} install

@@ -41,4 +40,7 @@
 %find_lang %{name}

+%clean
+rm -rf %{buildroot}
+
 %files -f %{name}.lang
 %{_mandir}/man1/blivet-gui.1*

- also please link the SPEC file directly (raw), not HTML.

- checksums of archive included in SRPM and upstream archive are different:
6e5dd8581fb8b73c17bd0981cd367a18  blivet-gui-0.2.0.tar.gz
9ded8f2490c7561adbfe170bc4736262  blivet-gui-blivet-gui-0.2.0.tar.gz

Also the provided source0 URL serves blivet-gui-blivet-gui-0.2.0.tar.gz, not
blivet-gui-0.2.0.tar.gz. Fix the source0 URL or add workaround to SPEC, e.g.:
https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Troublesome_URLs

- find_lang macro should be in %install section, not %check

- desktop_file_validate should be also in %install section, please use the
%check section only if there is working %check target in your makefile with
useful built-time tests

- in files section use macros, e.g. %{_datadir} for /usr/share, %{_bindir} for
/usr/bin

- you don't need to run 'rm -rf %{buildroot}' in %install section

- your package must not own /usr/share/applications and
/usr/share/polkit-1/actions directories, only own individual files/subdirs


Minor, mostly cosmetic issues:

- BuildRequires:intltool
  Please add space after colon to be consistent

- Url: http://github.com/rhinstaller/blivet-gui
  Please rather use URL: (i.e. in capitals).


Other:

- Is the source code python 3 ready? In such case you should package it
conditionally for python 3 (http://fedoraproject.org/wiki/Packaging:Python). If
not, it's OK as is.

- Source package should contains license text, as upstream please add it.

- Directories /usr/share/help and /usr/share/help/C are not owned, but this is
something you cannot resolve. AFAIK /usr/share/help is not in FHS standard.
This should be provided by some package. Question is which package, gtk?
gtk-doc? Any opinion? Bug should be filled against the identified package to
include these directories.

-- 
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