[Bug 1769843] Review Request: low-memory-monitor - Monitors low-memory conditions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1769843



--- Comment #7 from Bastien Nocera <bnocera@xxxxxxxxxx> ---
(In reply to Michael Catanzaro from comment #6)
> (In reply to Bastien Nocera from comment #5)
> > It's not really a configuration file, and shouldn't be modified by users, or
> > even
> > administrators.
> 
> I notice there is also %{_datadir}/dbus-1/system.d/. Not sure about the
> history behind these two locations, but how about we put it there instead?
> Seems like a better place?

Indeed, done upstream.

> > I would actually want the file to be replaced to automatically use the new
> > defaults
> > set in the package, rather than keep the old ones. So the current tag is
> > correct for
> > this file.
> 
> That is allowed if you add a comment to the spec file, but it's discouraged:
> 
> """
> As a rule of thumb, use %config(noreplace) instead of plain %config unless
> your best, educated guess is that doing so will break things. In other
> words, think hard before overwriting local changes in configuration files on
> package upgrades. An example case when /not/ to use noreplace is when a
> package’s configuration file changes so that the new package revision
> wouldn’t work with the config file from the previous package revision.
> Whenever plain %config is used, add a brief comment to the specfile
> explaining why.
> """
> 
> Basically users won't ever be able to safely edit this file if you replace
> their changes on package upgrade. Seems better for low-memory-monitor to
> assume default values for anything it doesn't find in its configuration
> file, which you probably do already anyway.
> 
> But if you really want to replace it despite that guidance, you can add a
> comment.

I've nuked the config file from the package. Now admins can create their own
config files if they want to override the default, but the default is in the
binary itself, controlled by the distributor.

> > That expands to:
> > %systemd_requires \
> > Requires(post): systemd \
> > Requires(preun): systemd \
> > Requires(postun): systemd \
> > %{nil}
> >
> > Which I think is what we want. Or am I missing something?
> 
> Final sentence here:
> 
> """
> If package scriptlets call other systemd tools, for example
> systemd-tmpfiles, the package SHOULD declare appropriate dependencies. The
> %systemd_requires macro is a shortcut to require systemd for the %pre,
> %post, and %postun scriptlets. Note that those dependencies are not required
> for the %systemd_{post,preun,postun_with_restart,user_post,user_preun}
> macros listed above.
> """

OK, will remove this.

I'll update the package soon.

-- 
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
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux