all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Stoiko Ivanov <s.ivanov@proxmox.com>
Subject: [pve-devel] applied-series: [PATCH common/manager v2] adapt PVE::Tools::sendmail to match rust-implementation and use it for apt update notifications
Date: Mon, 7 Sep 2020 10:06:01 +0200	[thread overview]
Message-ID: <4bf1dc8d-c7b4-25fa-2c4e-bcf2d465764d@proxmox.com> (raw)
In-Reply-To: <20200903120946.23596-1-s.ivanov@proxmox.com>

On 03.09.20 14:09, Stoiko Ivanov wrote:
> v1->v2:
> * dropped the moving of the email regex for reuse in the sendmail helper:
>   we use local usernames (mostly 'root') quite extensively in our code-base and
>   our users probably do so as well (for backup notifications)
> * replaced the direct invocation of /usr/sbin/sendmail by a call to the helper
>   in pve-manager/PVE/API2/APT.pm
> * adapted the sendmail-helper to allow for an empty display name in the from
>   header, since the apt update notification mail got sent without one.
> 
> The second patch in pve-common can be squashed into the first one, if
> preferred.

The separation in two patches as you did was good and warranted.

> 
> original cover-letter for v1:
> The 2 patches adapt PVE::Tools::sendmail to closely match the recently merged
> implementation in our rust repository - see [0].
> 
> I moved the email regex from JSONSchema to Tools to reuse it for the sendmail
> function (and eliminate one of the few email-address regexes in our codebase).
> 
> I did not add a dependency on libtimedate-perl (where Date::Format is), since
> we already use  Date::Parse in PVE::Certificate, without explicit dependency,
> and it gets pulled in via libwww-perl -> libhttp-date-perl -> libtimedate-perl.
> 
> Glad to send an update for the dependency of course.

should still note that, depending on indirect dependency chains isn't to ideal.

> 
> [0] https://lists.proxmox.com/pipermail/pbs-devel/2020-August/000423.html
> 
> Stoiko Ivanov (2):
>   move email regex from JSONSchema to Tools
>   sendmail-helper: only send multipart if necessary
> 
>  src/PVE/JSONSchema.pm |  4 ++--
>  src/PVE/Tools.pm      | 49 +++++++++++++++++++++++++++++--------------
>  2 files changed, 35 insertions(+), 18 deletions(-)
> 
> pve-common:
> Stoiko Ivanov (2):
>   sendmail-helper: only send multipart if necessary
>   sendmail helper: allow empty display name in from
> 
>  src/PVE/Tools.pm | 43 +++++++++++++++++++++++++++++--------------
>  1 file changed, 29 insertions(+), 14 deletions(-)
> 
> pve-manager:
> Stoiko Ivanov (1):
>   use PVE::Tools::sendmail for update notifications
> 
>  PVE/API2/APT.pm | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
> 



applied series, thanks!




      parent reply	other threads:[~2020-09-07  8:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-03 12:09 [pve-devel] " Stoiko Ivanov
2020-09-03 12:09 ` [pve-devel] [PATCH common v2 1/2] sendmail-helper: only send multipart if necessary Stoiko Ivanov
2020-09-03 12:09 ` [pve-devel] [PATCH common v2 2/2] sendmail helper: allow empty display name in from Stoiko Ivanov
2020-09-03 12:09 ` [pve-devel] [PATCH manager v2 1/1] use PVE::Tools::sendmail for update notifications Stoiko Ivanov
2020-09-07  8:06 ` Thomas Lamprecht [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4bf1dc8d-c7b4-25fa-2c4e-bcf2d465764d@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=s.ivanov@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal