https://bugzilla.redhat.com/show_bug.cgi?id=2007918
Jakub Kadlčík <jkadlcik(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jkadlcik(a)redhat.com
--- Comment #1 from Jakub Kadlčík <jkadlcik(a)redhat.com> ---
Hello Frank,
I like your project and the package as well.
%if 0%{?python3_pkgversion}
Requires: python%{python3_pkgversion}
%else
Requires: python3
%endif
According to the documentation
https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_depend...
A package should not specify its dependency on python3 package. I
think you can drop those lines.
BuildRequires: sed python3-rpm-macros python-srpm-macros
I cannot find the proper documentation right now but I believe you
don't have to explicitly depend on `sed` as well because it is a part
of the minimal buildroot.
Also, I would recommend specifying one dependency per line. Like this:
BuildRequires: sed
BuildRequires: python3-rpm-macros
BuildRequires: python-srpm-macros
Later on, it will be much easier to read the diffs when some
dependency gets removed/added. This is just a tip from personal
experience, and it is totally up to your preference. The way you did
it is also correct.
Tuptime track and report historical and statistical real time of the
system, keeping the uptime and downtime between shutdowns.
The space at the beginning of the second line is unnecessary
%build
exit 0
You can also leave the %build phase empty
cp -R %{_topdir}/BUILD/%{name}-%{version}/src/tuptime
%{buildroot}%{_bindir}/
...
You don't have to specify the absolute path to the sources. You can
simply use
cp -R src/tuptime %{buildroot}%{_bindir}/
Also, using -R for files doesn't have any benefits and makes it a
little bit harder to read.
%{_mandir}/man1/tuptime.1.gz
I would recommend using the following wildcard instead
%{_mandir}/man1/tuptime.1.*
From time to time, it happens that a compression format for something
is changed, and this way you won't have to deal with that. Every
package I know uses this :-)
%doc misc/scripts/
Those scripts don't look like they are used for documentation
purposes but rather as something that is supposed to be executed
between major upgrades. I think they should be stored somewhere else
than the /usr/share/doc directory
%defattr(-,root,root)
%dir %attr(0755, _tuptime, _tuptime) %{_sharedstatedir}/tuptime/
Is there any motivation for setting these permissions? AFAIK they are
the defaults because dropping these two lines don't seem to have any
effect on the output.
su -s /bin/sh _tuptime -c "(umask 0022 &&
/usr/bin/tuptime -x)"
I am not sure what this line does. Can you please help me understand?
I made a couple of suggestions. They are just minor details that are
easy to fix. Overall, I really like the package. Good job here, Frank.
--
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
https://bugzilla.redhat.com/show_bug.cgi?id=2007918