Re: [PATCH v3 0/1] MEDIUM: mworker: Add systemd `Type=notify` support

2017-11-20 Thread Willy Tarreau
On Mon, Nov 20, 2017 at 03:58:34PM +0100, Tim Düsterhus wrote:
> Coming along is a patch that implements your suggested changes.
> 
> Two things:
> - I was unsure whether to renumber the GTUNE_* constants. I opted not to.

Not needed. It might be a cleanup for later. However I moved your decalaration
(1<<10) after (1<<9) to avoid the risk that someone else spots the empty slot
and reuses it.

> - I was unsure whether `#if defined(...)` or `#ifdef` is the variant to use
>   for the USE_SYSTEMD option. You use both and I was unable to find out
>   whether there is a scheme you follow.

The two are valid. Most often ifdef is used, but when you want to apply
combinations, if defined() often becomes more convenient.

> Thanks for your patience and really helpful feedback so far :-)

You're welcome, thanks for working on this. I've applied your patch with
William's blessing :-)

Willy



[PATCH v3 0/1] MEDIUM: mworker: Add systemd `Type=notify` support

2017-11-20 Thread Tim Düsterhus
Willy,

> I really don't like this at all, because it means that we'll annoy all
> the happy people who are not forced to suffer from systemd, ie basically
> all those not running on a recent linux distro, simply because we're starting
> to declare that certain environment variables only belong to other operating
> systems (which is even worse when such variables are detected when the build
> option is *not* set). Also the fact that such variables could be inherited
> across multiple layers of processes is even more disgusting.
> *snip*

Understood. Coming along is a patch that implements your suggested changes.

Two things:
- I was unsure whether to renumber the GTUNE_* constants. I opted not to.
- I was unsure whether `#if defined(...)` or `#ifdef` is the variant to use
  for the USE_SYSTEMD option. You use both and I was unable to find out
  whether there is a scheme you follow.

Thanks for your patience and really helpful feedback so far :-)

Tim Duesterhus (1):
  MEDIUM: mworker: Add systemd `Type=notify` support

 Makefile   |  7 +++
 contrib/systemd/haproxy.service.in |  4 ++--
 include/types/global.h |  1 +
 src/haproxy.c  | 29 +
 4 files changed, 39 insertions(+), 2 deletions(-)

-- 
2.15.0