[Bug 692069] Review Request: pps-tools - LinuxPPS user-space tools
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=692069 --- Comment #5 from Miroslav Lichvar mlich...@redhat.com 2011-08-10 09:42:59 EDT --- I think relative symlinks are preferred for this. Thanks for the review! -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 692069] Review Request: pps-tools - LinuxPPS user-space tools
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=692069 Miroslav Lichvar mlich...@redhat.com changed: What|Removed |Added Flag||fedora-cvs? --- Comment #6 from Miroslav Lichvar mlich...@redhat.com 2011-08-10 09:45:44 EDT --- New Package SCM Request === Package Name: pps-tools Short Description: LinuxPPS user-space tools Owners: mlichvar Branches: f15 f16 InitialCC: -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 692069] Review Request: pps-tools - LinuxPPS user-space tools
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=692069 --- Comment #7 from Jon Ciesla l...@jcomserv.net 2011-08-10 11:09:59 EDT --- Git done (by process-git-requests). -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 692069] Review Request: pps-tools - LinuxPPS user-space tools
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=692069 Ankur Sinha sanjay.an...@gmail.com changed: What|Removed |Added Status|NEW |ASSIGNED CC||sanjay.an...@gmail.com AssignedTo|nob...@fedoraproject.org|sanjay.an...@gmail.com Flag||fedora-review? -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 692069] Review Request: pps-tools - LinuxPPS user-space tools
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=692069
--- Comment #2 from Ankur Sinha sanjay.an...@gmail.com 2011-08-09 12:05:25
EDT ---
Review:
+ OK
- NA
? ISSUE
+ Package meets naming and packaging guidelines
+ Spec file matches base package name.
+ Spec has consistant macro usage.
+ Meets Packaging Guidelines.
+ License
+ License field in spec matches
? License file included in package
^^
Please include debian/copyright in the package's %doc section.
Is the README worth including in %docs.
+ Spec in American English
+ Spec is legible.
+ Sources match upstream md5sum:
^^^
git archives, checked with diff:
[ankur@ankur pps-tools]$ pwd
/home/ankur/dump/pps-tools
[ankur@ankur pps-tools]$ diff -ur ../../rpmbuild/SOURCES/pps-tools/ ./
Only in ./: .git
- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
+ Package has a correct %clean section.
+ Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
^^
If you're not building for rhel etc., you can get rid of the above 3 portions.
+ Package is code or permissible content.
- Doc subpackage needed/used.
+ Packages %doc files don't affect runtime.
+ Headers/static libs in -devel subpackage.
^^
Even though there's only one header, I think we should leave it in the -devel
package.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
? -devel package Requires: %{name} = %{version}-%{release}
^^
The devel is only a header. No sonames or anything here. Don't think this is
required. Need to confirm.
- .la files are removed.
- Package is a GUI app and has a .desktop file
+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
+ No rpmlint output.
+ final provides and requires are sane:
== pps-tools-0-0.1.20100413git74c32c.fc17.i686.rpm ==
Provides:
pps-tools = 0-0.1.20100413git74c32c.fc17
pps-tools(x86-32) = 0-0.1.20100413git74c32c.fc17
Requires:
/bin/sh
libc.so.6
libc.so.6(GLIBC_2.0)
libc.so.6(GLIBC_2.3.4)
rtld(GNU_HASH)
== pps-tools-0-0.1.20100413git74c32c.fc17.src.rpm ==
Provides:
Requires:
== pps-tools-debuginfo-0-0.1.20100413git74c32c.fc17.i686.rpm ==
Provides:
pps-tools-debuginfo = 0-0.1.20100413git74c32c.fc17
pps-tools-debuginfo(x86-32) = 0-0.1.20100413git74c32c.fc17
Requires:
== pps-tools-devel-0-0.1.20100413git74c32c.fc17.i686.rpm ==
Provides:
pps-tools-devel = 0-0.1.20100413git74c32c.fc17
pps-tools-devel(x86-32) = 0-0.1.20100413git74c32c.fc17
Requires:
SHOULD Items:
+ Should build in mock.
+ Should build on all supported archs
^^ builds on both i386 and x86_64
- Should function as described.
- Should have sane scriptlets.
+ Should have subpackages require base package with fully versioned depend.
Not required for this package.
+ Should have dist tag
+ Should package latest version
- check for outstanding bugs on package. (For core merge reviews)
Issues:
1. Only the license/docs need to be included.
--
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 692069] Review Request: pps-tools - LinuxPPS user-space tools
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=692069 --- Comment #3 from Miroslav Lichvar mlich...@redhat.com 2011-08-09 12:49:05 EDT --- Thanks for the review. I've updated the spec to include the README and copyright files and also included a symlink in /usr/include/sys for better compatibility. The devel package shouldn't need anything from the base package. http://mlichvar.fedorapeople.org/tmp/pps-tools-0-0.2.20100413git74c32c.fc14.src.rpm -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 692069] Review Request: pps-tools - LinuxPPS user-space tools
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=692069
Ankur Sinha sanjay.an...@gmail.com changed:
What|Removed |Added
Flag|fedora-review? |fedora-review+
--- Comment #4 from Ankur Sinha sanjay.an...@gmail.com 2011-08-09 13:08:54
EDT ---
Hello,
I was just wondering if it would be better to link it like this:
ln -s $RPM_BUILD_ROOT%{_includedir}/timepps.h $RPM_BUILD_ROOT%{_includedir}/sys
instead of
ln -s ../timepps.h $RPM_BUILD_ROOT%{_includedir}/sys
The rest looks good!
[ankur@ankur SRPMS]$ rpmlint /var/lib/mock/fedora-rawhide-i386/result/*.rpm
pps-tools.i686: W: no-manual-page-for-binary ppsbind
pps-tools.i686: W: no-manual-page-for-binary ppswatch
pps-tools.i686: W: no-manual-page-for-binary ppsfind
pps-tools.i686: W: no-manual-page-for-binary ppstest
pps-tools.src: W: invalid-url Source0: pps-tools-20100413git74c32c.tar.gz
pps-tools-debuginfo.i686: E: incorrect-fsf-address
/usr/src/debug/pps-tools/timepps.h
pps-tools-devel.i686: W: no-documentation
pps-tools-devel.i686: E: incorrect-fsf-address /usr/include/timepps.h
4 packages and 0 specfiles checked; 2 errors, 6 warnings.
Please do request upstream to correct the FSF address.
XX APPROVED XX
--
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 692069] Review Request: pps-tools - LinuxPPS user-space tools
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=692069 --- Comment #1 from Miroslav Lichvar mlich...@redhat.com 2011-03-30 07:06:28 EDT --- The -devel subpackage contains only one file, the timepps.h header and there are no dependencies, maybe it could go to the main package? The header file is a build requirement for ntp, chrony and gpsd packages, which currently include their own copy of the file. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
