[Bug 488100] Review Request: firebird - Firebird SQL database management system

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

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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





--- Comment #25 from Peter Lemenkov <lemenkov@xxxxxxxxx>  2009-03-28 03:59:07 EDT ---
Notes:

* These defines seems redundant:

%global realname                Firebird
%global major                   2.1.1.17910
%global minor                   0
%global realversion             %{major}-%{minor}

%realname used only with %realversion, which in turn is the only place where
%major and %minor are used.

I suggest you, to replace these macros with single one (say, %tarsourcedir or
something similar, which will be more descriptive and clear to others).

* I can't uderstant, why you defined a macro %bindir at line 2. Please add
explanation and/or remove it.

* You should explicitly define from which codepage you're converting, while
using iconv. E.g.

iconv -f <original codepage> -t utf-8

instead of

iconv -t utf-8 only.

* lines 187,259,260 looks like a leftover.

* line 244 MUST be removed - there is no need in such stupid tests anymore.

* Regarding %fbroot macro. I don't like the idea of installing *-devel files
into %libdir.

* I, personally, don't like naming scheme at all. I suspect that adding
lib-prefixes and "2" as postfix for library-subpackages  is redundant. Ad one
more - why so many servers (classic, super and related subpackages) and related
utils-subpackages? However, this may be generally accepted naming scheme among
firebird-users, but in any case - please explain it. 


I'll add more notes later.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

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