[Bug 1441816] Review Request: diorite - Utility and widget library for Nuvola Player

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

 



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



--- Comment #4 from mgansser@xxxxxxxx <mgansser@xxxxxxxxx> ---
(In reply to Vít Ondruch from comment #3)
> Several notes:
> 
> * Is there any reason to package the git snapshot? Wouldn't it be easier to
> go with the latest official release (0.3.2). I am using some older version
> (0.3.1) of your diorite package and I don't observe any issues. Also, since
> there is not provided any API/ABI stability guarantee, I'd say that it would
> be better to package just stable versions and the projects using this
> library should always explicitly specify the diorite version.

I will take the stable diorite-0.3.1 now
> 
> * I don't think the "Release" field is formatted according to the guidelines
> [1]. I should be 0.1.20171104git%{shortcommit0} IMO.
> 
> * What is the reason to use the bundled version of "waf"? We have waf
> package in Fedora, is it possible to use it instead?

I know this, but diorite need version 1.7.14 of waf, so i will use the bundle
version. Fedora have version 1.9.7.
> 
> * Wouldn't be %setup sufficient in place of %autosetup?

i will take %setup because no patches are needed.
> 
> * As far as I understand, the "waf configure" is using pkgconfig to check
> the proper configuration. Wouldn't it be more appropriate to use virtual
> provides such as "BuildRequires: pkgconfig(gtk+-3.0)"

i will change it to:
BuildRequires:  gcc
BuildRequires:  pkgconfig(python3)
BuildRequires:  pkgconfig(gtk+-3.0)
BuildRequires:  %{_bindir}/valac
> 
> * I don't think you need vala-devel, since you are not going to extend Vala.
> The only thing you need is actually Vala compiler which is provided by
> "vala" package, but it might be better to "BuildRequires:  %{_bindir}/valac"
> instead.

done
> 
> * Since the application is later build using GCC, you should consider to add
> "BuildRequires: gcc" [2].

done
> 
> * There appears to be tests suite. Is there a chance to execute it during
> build?

don't know what is the correct command ?

> 
> * The license should be just "BSD" shouldn't it? I can't see any code
> licensed under LGPLv3+ and LGPLv2+.

will change it to BSD only.
> 
> * There is no license file included. Could you please query upstream to add
> one [3]?

opened upstream ticket.
https://github.com/tiliado/diorite/issues/10

-- 
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
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




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