Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: alliance - Alliance VLSI CAD Sytem https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248649 ------- Additional Comments From cgoorah@xxxxxxxxxxxx 2007-07-18 05:30 EST ------- (In reply to comment #3) > Ok so few comments to start: > 1 / prefix seems problematic. But since no arch dependant files are installed in > it, this could be fine if we can have configs files in /etc actually... > Also no %config(no replace) seems to be used for users config files... Would you like me to put configs files in /etc and just create a symbolic link to /usr/share/alliance/etc ? Note: without this link some %_bindir/* would be broken since this path is hardcoded into the sources. > #2 #3 Ok, will change appropriately. > 4 / patches macros. > - As the pacakge name is good why do you need to uses %{name}? this bring some > confusion when looking for the patch instead of having the full name. > - Some patches are backport from older version (+ is older than -) This mean > that some patches could be usefull with later release ? I cannot see the aim of > using %{version} in this case! Unless it will break patch historicy in cvs if no > changes are made to the patch for later releases. They will break in cvs since upstream Patch0: %{name}-%{version}-addphcon.patch There are 2 bugs in this release: (on ocp and on boog) As for ocp, I've backported to the old release (20060218), till there is a fix. Concerning boog, upstream will be digging on it this week: https://www-asim.lip6.fr/wws/arc/alliance-users/2007-07/msg00017.html Patch1: %{name}-%{version}-examples.patch (is for the documentation/alliance-examples folder) Patch2: %{name}-%{version}-run.patch (is for the documentation/alliance-run folder) Patch3: %{name}-%{version}-perms.patch (setting the proper permissions) will be without %{version}. > 5 / desktop files > * You missed > Requires(post): desktop-file-utils > Requires(postun): desktop-file-utils As agreed if MimeType key in desktop files is used, and will be using in the next release. > * I don't know if the shortcuts will be in the right category... You mean in the menus ? > 6 / About the kindly requested > https://www.redhat.com/archives/fedora-devel-list/2007-July/msg00750.html > Why do you choose not to show the "kindly requested" in %description ? There is no big reason behind it, except I had already agreed with upstream on a separate file before Tom proposed. The %description would be too long. I've included it into a separate file alliance.fedora. If you want me to change accordingly, I can do it. > 7 / %configure > * This package do not conform to the standard paths and use a prefix with > --with-alliance-top=%{prefix}. But, do you need to export it to make it work ? You mean - export ALLIANCE_TOP=%{prefix} - %configure --prefix=%{prefix} --enable-alc-shared --disable-static + %configure --prefix=%{prefix} --enable-alc-shared \ --disable-static --with-alliance-top=%{prefix} ? > * --disable-static is avaible why don't you uses it ? Does it works ? will use > 8 / # applying timestamps > What do you mean by this ? This could go in %prep for Source7 will move to %prep > 9 / # documentation > Why do you copy them it "." ? (you do not seems to use them after that...) I need them in the -doc package. > 10 / #conflicts with man-pages and is a duplicate of log.1.gz > This make rise the problem of too much generic names appear (Which I haven't > checked yet). Maybe a renamed could be enought if the --program-prefix do not > work if this apply. It is a duplicate of log.1.gz as well. I see no harm removing a duplicate. > 11 / scriplets > * %preun -p /sbin/ldconfig - This is unneeded > * Recommand to have this if desktop file has a MimeType key. > %post > /sbin/ldconfig > update-desktop-database &> /dev/null || : > > %postun > /sbin/ldconfig > update-desktop-database &> /dev/null || : Will use MimeType key > 12 / # duplicate and unstripped-binary-or-object > %exclude %{_libdir}/debug > * This is wrong on x86_64 and also uneeded (tested) On i386, I have debug files before the script /usr/lib/rpm/find-debuginfo.sh Is there a way to disable this thing and let /usr/lib/rpm/find-debuginfo.sh do the job ? > 13 / %{_includedir}/* > * header are presents in main but not in devel - Is it possible to sort those > that should be used at runtime from those that are needed for developping alliance ? will try > 14 / %{_mandir}/man?/* > * Check if some of them shouldn't go in -devel will try > 15 / #Makefiles are present in alliance-examples/* > * Is it possible to have another sub-package for these examples (which will > follow others rules of Requirement eventually ) > * Having users to build them is %doc directory is not fair - Thoses can go in > %{_datadir}/alliance/examples. Ok, will be opting for a "alliance-examples" sub package. > 16 / build fails on x86_64 FC6: (i will give a retry ) > /usr/bin/ld: cannot find -lMvg > collect2: ld returned 1 exit status > make[2]: *** [x2vy] Error 1 > make[2]: *** Waiting for unfinished jobs.... Are you using %{__make} %{?_smp_mflags} ? -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review