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

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

 



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



--- Comment #4 from Vojtech Trefny <vtrefny@xxxxxxxxxx> ---
(In reply to Jaroslav Škarvada from comment #3)
> I will sponsor you after we finish the review.

New spec/srpm:

https://vtrefny.fedorapeople.org/spec/blivet-gui.spec
https://vtrefny.fedorapeople.org/srpms/blivet-gui-0.2.0-5.fc21.src.rpm

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

The problem here is I don't have localisation files in git repository -- MO
files are in SPRM because I download them from Zanata during "make archive"
(and upstream archives are made automatically by GitHub for every tag).

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

Fixed.

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

Fixed.

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

Fixed.

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

Not it isn't because python-blivet is not python3 read yet.

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