Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tmpwatch https://bugzilla.redhat.com/show_bug.cgi?id=226495 ------- Additional Comments From mitr@xxxxxxxxxx 2008-01-17 21:37 EST ------- Created an attachment (id=292099) --> (https://bugzilla.redhat.com/attachment.cgi?id=292099&action=view) Spec file for tmpwatch-2.9.12-3 Thank you both for the reviews. (In reply to comment #1) > * Parallel make: > Append to make %{?_smp_mflags} in %{build}, unless you have a reason not to do > so (package does not build with parallel make). It does not make any difference right now, because (make) runs only one command - but future-proofing the spec file doesn't hurt. Fixed. > * Macro usage: > > Use %{_sysconfdir} instead of /etc I'm not sure it makes sense for /etc/cron.daily; I understand one can override %{_prefix} and have a special set of packages rooted e.g. under /opt/fedora-backports, but cron.daily/tmpwatch must be in a directory that is actually processed by the system crontab, not under /opt/fedora-backports. This is all hypothetical though, I'm quite willing to use the macro if you think it is necessary. (In reply to comment #2) > I there's really no upstream, please make a note of that in the spec. > http://fedoraproject.org/wiki/Packaging/SourceURL Added. > If it's hosted internally to Red Hat, please consider transferring it to some > externally visible site such as fedorahosted.org. It is actually on elvis. I should really move all my packages to hosted, but there's always some more urgent work. > Parallel make: there's only one source file so there's really no point, but if > you're fixin things you might as well future-proof thins. Added. > Then there's this rpmlint complaint: > %{_sysconfdir} is generally preferred to /etc. See above. > tmpwatch.x86_64: E: executable-marked-as-config-file /etc/cron.daily/tmpwatch > This one is interesting. I'm guessing that this was marked as %config in the > past because someone was annoyed that a package update wiped out their changes. > However: > A shell script is a really poor configuration interface. > If we end up having to protect an additional directory in /tmp from cleanup, > there will be issues because we can't update that script. > > Would it be at all possible to move the bits that people might want to configure > into a file in /etc/sysconfig? It doesn't look like it should be difficult at all. People "might want to configure" most of the script (it would be "reasonable" to add at least 6 variables), and I'm afraid I don't know how to make the script configurable in a reasonable future-proof way that wouldn't encourage adding progressively convoluted variables every few months, and wouldn't be horribly overengineered. > The only licensing information I can see is in tmpwatch.c, which says only > "under the terms of the GPL". Isn't Red Hat developed code supposed to be GPLv2 > only? I'm told it's corporate policy. Yes; I guess this was an oversight - but over time, patches from external contributors were accepted and I don't think this would be fair to them. I'll ask our lawyers. > BuildRoot: is not correct; it needs to have at least %release, but would be > better if it were it were one of the recommended values. Fixed. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review