Subject: Re: bug#31498: [PATCH] Notify systemd in daemon-initialized

From: Lucas Werkmeister <[email-masked]>
Date: Thu, 7 Jun 2018 23:40:08 +0200

Thanks for the review! Replies inline.

On 07.06.2018 15:08, Noam Postavsky wrote:
> severity 31498 wishlist
> severity 31521 wishlist
> quit
> 
> 
> Thanks, the patch seems straightforward enough.  Just a few minor
> comments about formatting.  Have you assigned copyright for Emacs?  The
> patch is small enough to go in anyway, it would just have to be marked.

I haven’t assigned copyright yet, but I’d be happy to do so – I’ve tried
to get the process started on emacs-devel.

> 
> Lucas Werkmeister <[email-masked]> writes:
> 
>> With --[bg-]daemon and Type=forking, systemd will only consider the
>> daemon to have fully started up once the original process exits, and
>> will wait until then to start units depending on the Emacs service. To
>                                                                      ^
>                                                                      double space

ack

>> get the same functionality with --fg-daemon, use Type=notify instead of
>> Type=simple and explicitly send a readiness notification to systemd at
>> the point where the forked process would in --bg-daemon mode notify its
>> parent process and cause it to exit.
>> * etc/emacs.service: update Type
> 
> The ChangeLog entries should be captilalized and end with a period:
> "Update Type."
> 
>> * src/emacs.c (daemon-initialized): call sd_notify()
> 
> Same here.

2× ack

> 
> I think it should have a NEWS entry mentioning that Emacs now supports
> this systemd notification feature.  I would probably go ahead and
> combine with #31521, since it's using the same feature.
> 

Do you mean, announce both in this patch, or completely merge that patch
into this one?

Once I’ve fixed the problems locally, what’s the best way to re-roll the
patch in this bug tracking system? Should I try to configure `git
send-email` to reply to the right email (and with the right bug in CC),
or perhaps use `git format-patch` and add the patch file as a regular
attachment in a manual response?