Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: comex-base - base component for comex project
https://bugzilla.redhat.com/show_bug.cgi?id=735944
Summary: Review Request: comex-base - base component for comex project Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: hmandevteam@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
Spec URL: http://www.integrazioneweb.com/repository/specs/fedora/comex-base.spec SRPM URL: http://www.integrazioneweb.com/repository/srpms/fedora/comex-base-0.1.7.0-1.... Description: Is base component of a simple application that can be used to exchange data with smartcards using PC/SC standard readers or smartmouse phoenix serial reader.
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=735944
--- Comment #1 from Armando Basile hmandevteam@gmail.com 2011-09-07 17:55:24 EDT --- Spec URL: http://www.integrazioneweb.com/repository/specs/fedora/comex-base.spec SRPM URL: http://www.integrazioneweb.com/repository/srpms/fedora/comex-base-0.1.7.0-2....
changes: - added ExclusiveArch - added pcsc-lite, pcsc-lite-devel, pcsc-lite-libs dependency - added %{?_smp_mflags} to make command
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=735944
--- Comment #2 from Mamoru Tasaka mtasaka@fedoraproject.org 2011-09-08 16:00:03 EDT --- Well, only did a very quick check
* License tag - GPLv2 is chosen for license tag in your spec, however I cannot see any files which specifies the version of GPL in the source. Note that just including GPLv2 license text does not actually specify the version of GPL used in the source.
Currently License tag should be "GPL+"
* BuildRequires - Currently your srpm won't build on koji. Please make it sure that your srpm builds on koji (or with mockbuild) http://koji.fedoraproject.org/koji/taskinfo?taskID=3336036
It seems mono-devel is needed for BR
* Explicit dependency - Note that with adding "BR: mono-devel", rpmbuild will find out mono-related runtime dependency (i.e. Requires) automatically and adds the detected dependencies to binary rpms. So writing "Requires: mono-core log4net" explicitly is not needed (after adding BR: mono-devel)
* Requires for main pacakge - Usually Requires for main package on subpackages should be (Epoch:)Version-Release specific and also isa specific
https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package
* pkgconfig dependency - Explicit "Requires: pkgconfig" is no longer needed on Fedora, it is added automatically by rpmbuild https://fedoraproject.org/wiki/Packaging/Guidelines#Pkgconfig_Files
* Deprecated usage Please remove lines which are no longer needed - %if 0%{?fedora} < 13 condition is useless. Fedora's version is at least 14 currently - The following is no longer needed on Fedora (EPEL may still need these) * BuildRoot: line * rm -fr %{buildroot} at the top of %install * %clean section
* Directory ownership issue - The directory %{_libdir}/mono/gac/ itself is owned by mono-core and this package should not own this directory itself. - The directory %{_datadir}/%{name}/ is not owned by any packages and this package should own it
* Requires: pcsc-lite-devel - Can't this be avoided by patching comex-base/comex-base.dll.config?
Note that I will be away till Sunday so it may be that replying to you becomes late. By the way I will appreciate it if you would review my review request (bug 678674)
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=735944
--- Comment #3 from Armando Basile hmandevteam@gmail.com 2011-09-11 16:56:27 EDT --- SPEC: http://www.integrazioneweb.com/repository/specs/fedora/comex-base.spec SRPMS: http://www.integrazioneweb.com/repository/srpms/fedora/comex-base-0.1.7.1-1....
changes in sources http://code.google.com/p/comex-project/source/detail?r=73: - removed dependence from pcsc-lite devel package - changed alias in .config file (now also .Net see library) - changed DllImport alias for Interop (now also .Net see library) - added license information
changes in spec: - in BuildRequires: changed mono-core to mono-devel - in Requires: removed pcsc-lite-devel - in Requires: removed mono-core - in Requires: removed log4net - in Requires: removed pkgconfig - build section: removed "if fedora < 13" condition - removed BuildRoot - install section: removed "rm -fr %{buildroot}" - clean section: removed - files section: changed ref from mono/gac/ to mono/gac/comex-base/ - files section: changed ref from %{name}/Languages/ to %{name}/
about your review request bug 678674, i don't know ruby so i don't know how could i help you.
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=735944
--- Comment #4 from Mamoru Tasaka mtasaka@fedoraproject.org 2011-09-12 12:09:03 EDT --- Well, I have not checked your latest srpm, however just for this:
(In reply to comment #3)
about your review request bug 678674, i don't know ruby so i don't know how could i help you.
Note that I don't know mono well, either :)
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=735944
--- Comment #5 from Armando Basile hmandevteam@gmail.com 2011-09-12 12:29:34 EDT ---
Note that I don't know mono well, either :)
Wow, seems you are familiar with mono
Could try to review a mono package for first ?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=735944
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(hmandevteam@gmail | |.com)
--- Comment #6 from Jason Tibbitts tibbs@math.uh.edu --- I am triaging old review tickets. I can't promise a review if you reply, but by closing out the stale tickets we can devote extra attention to the ones which aren't stale.
spec and srpm links are invalid.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=735944
Armando Basile hmandevteam@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(hmandevteam@gmail | |.com) |
--- Comment #7 from Armando Basile hmandevteam@gmail.com --- i updating specs and srmps, thanks for reply
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=735944
--- Comment #8 from Armando Basile hmandevteam@gmail.com --- SPEC: http://www.integrazioneweb.com/repository/specs/fedora/comex-base.spec SRPMS: http://www.integrazioneweb.com/repository/srpms/fedora/comex-base-0.1.8.5-1....
changes in spec: - in build section: set --libdir=%{_prefix}/lib (http://fedoraproject.org/wiki/Packaging:Mono#File_Locations)
https://bugzilla.redhat.com/show_bug.cgi?id=735944
Mario Blättermann mario.blaettermann@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mario.blaettermann@gmail.co | |m
--- Comment #9 from Mario Blättermann mario.blaettermann@gmail.com --- Some initial comments before I give it a try on Koji:
Probably no debuginfo will be created, due to the nature of Mono software: https://fedoraproject.org/wiki/Packaging:Debuginfo?rd=Packaging/Debuginfo#Us... This is addressed by your comment in spec line 13. OK so far, but either you uncomment the line (if debuginfo will be usable in this certain case) or you have to escape the % signs with another one to make rpmlint happy. In some odd cases, even macros in comments could be expanded and lead to unexpected results.
Requires: %{name} = %{version} has to be Requires: %{name}%{?_isa} = %{version}-%{release} The isa tag needs to be added because your package is multiarch. The release tag makes sure that the -devel package will be updated with the base package.
%defattr(-,root,root,-) is applicable for EPEL 5 only. As the second changelog entry says, you don't plan to package for EPEL 5, please remove that lines in the file lists.
In the file lists, %{_prefix}/lib has to replaced with %{_libdir}. Otherwise we would get a "hardcoded library path" warning from rpmlint. Or can you explain why you have to use /usr/lib explicitely instead of /usr/lib and /usr/lib64 depending on the package architecture?
%changelog * Thu May 02 2013 Armando Basile hmandevteam@gmail.com 0.1.8.5-1.fc18 In the %changelog section, it is unneeded to mention the distribution release. A simple "0.1.8.5-1" is sufficient here. Doesn't matter which Fedora release is pointed by your spec, one could rebuild your package for f17 or f19, whatever, so that your "f18" tag becomes useless.
Requires: pcsc-lite Requires: pcsc-lite-libs The latter is not needed, it is required by pcsc-lite anyway.
You are using both tabs and whitespaces in your spec. It's rather cosmetic, but has to be fixed. I recommend spaces, that way the spec looks the same in any text editor, regardless of the configured tab width.
https://bugzilla.redhat.com/show_bug.cgi?id=735944
Mario Blättermann mario.blaettermann@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hmandevteam@gmail.com Flags| |needinfo?(hmandevteam@gmail | |.com)
--- Comment #10 from Mario Blättermann mario.blaettermann@gmail.com --- Any news here...?
https://bugzilla.redhat.com/show_bug.cgi?id=735944
Mario Blättermann mario.blaettermann@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC|mario.blaettermann@gmail.co | |m | Flags|needinfo?(hmandevteam@gmail | |.com) |
https://bugzilla.redhat.com/show_bug.cgi?id=735944
--- Comment #11 from Armando Basile hmandevteam@gmail.com --- Hi Mario, i will modify specs and resubmit, thanks
https://bugzilla.redhat.com/show_bug.cgi?id=735944
Miroslav Suchý msuchy@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED CC| |msuchy@redhat.com Resolution|--- |INSUFFICIENT_DATA Last Closed| |2015-08-21 05:22:06
--- Comment #12 from Miroslav Suchý msuchy@redhat.com --- No response for years. Closing. Feel free to reopen if you want to continue.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla