Code review comment for ~bryce/git-ubuntu:sendmail-object

Revision history for this message
Robie Basak (racb) wrote :

I don't see review feedback incorporated. Did you push the right branch? I'm looking at efea26f. Here are the changes I see against what I reviewed previously:

$ git range-diff d358c19..bryce/sendmail-object.v1 9e8e5da..bryce/sendmail-object
1: bb91757 = 1: 88a6898 Refactor importer to encapsulate sendmail logic to its own object
2: 6e038ff = 2: 4a546e4 Send fewer package import/failure emails
3: b361f4b ! 3: efea26f systemd: Include last 100 lines from journal in system failure emails
    @@ -2,6 +2,11 @@

         systemd: Include last 100 lines from journal in system failure emails

    + Log errors from this script to the journal as well.
    +
    + Also, drop unnecessary User/Group settings in service, since this runs
    + fine as the current (non-root) user.
    +
     diff --git a/bin/failure-email.sh b/bin/failure-email.sh
     --- a/bin/failure-email.sh
     +++ b/bin/failure-email.sh
    @@ -19,3 +24,17 @@
     +$(journalctl --unit=usd-importer.service -n 100 --no-pager)
      EOF

    +
    +diff --git a/usd-importer-failure-email@.service b/usd-importer-failure-email@.service
    +--- a/usd-importer-failure-email@.service
    ++++ b/usd-importer-failure-email@.service
    +@@
    +
    + [Service]
    + Type=oneshot
    +-ExecStart=/snap/git-ubuntu/current/bin/failure-email.sh %i
    +-User=nobody
    +-Group=systemd-journal
    ++ExecStart=/bin/bash /snap/git-ubuntu/current/bin/failure-email.sh %i
    ++StandardOutput=journal
    ++

review: Needs Fixing

« Back to merge proposal