servo / saltfs

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For Issue #413 - WIP for configuring failed build notification #595

Closed

matt-duell wants to merge 4 commits into servo:master from matt-duell:master

+22 −10

matt-duell commented Feb 4, 2017 •

edited by larsbergstrom

I hope this is the direction people were hoping for.
Obviously not complete, the emails are just dummy ones.

Going off of the suggestion made by @aneeshusa , I added a list of builders which this configuration applies for. This would allow for someone to disable notifications for certain builds.

This change is


        For Issue #413 - general idea on configuring the mail notifier for fa…

194095f

…iled builds. Still requires proper emails

highfive assigned aneeshusa

highfive commented Feb 4, 2017

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @aneeshusa (or someone else) soon.

highfive added the S-awaiting-review label


        Removing lines containing whitespace failing build.

05e0a47

aneeshusa suggested changes

Member

aneeshusa left a comment

Looks good so far! Looking at the docs for MailNotifier, I'd like to add a few more options:

set mode: ['failing', 'exception']
use addPatch: False, since nightlies aren't tied to a specific PR

Also, the default configuration seems to rely on a SMTP server running on localhost port 25, which we don't have. cc @edunham any ideas about SMTP setup for this?

If SMTP is difficult to get working, we could try notifying in IRC.

buildbot/master/files/config/master.cfg Outdated

  
      @@ -13,7 +14,6 @@ MAC_SLAVES = ["servo-mac2", "servo-mac3", "servo-mac4", "servo-macpro1"]
    
        CROSS_SLAVES = ["servo-linux-cross{}".format(i) for i in range(1, 3)]

        WINDOWS_SLAVES = ["servo-windows{}".format(i) for i in range(1, 3)]

aneeshusa Feb 7, 2017

Member

Please leave this line.

buildbot/master/files/config/master.cfg Outdated

  
                fromaddr="buildbot@example.org",

                sendToInterestedUsers=False,

                extraRecipients=["listaddr@example.org"],

                builders=[

aneeshusa Feb 7, 2017

Member

I know in the issue I mentioned it would be nice to easily disable builders, but now that Android is back up and running I'm more concerned about this list getting out of sync with the actual nightly builders list. Can you deduplicate these, either by pulling out the list itself or accessing some property on c['schedulers']?

buildbot/master/files/config/master.cfg Outdated

  
                    "mac-nightly",

                    "mac-rel-intermittent",

                    "windows-gnu-nightly",

                    "windows-msvc-nightly"

aneeshusa Feb 7, 2017

Member

nit: Add a trailing comma

buildbot/master/files/config/master.cfg Outdated

  
                    "windows-gnu-nightly",

                    "windows-msvc-nightly"

                ]

            )

aneeshusa Feb 7, 2017

Member

nit: Add a trailing comma

aneeshusa added S-awaiting-answer S-needs-code-changes and removed S-awaiting-review labels


        Implemented feedback from PR. centralized nightly builders list.

0b6d4a1

highfive added S-awaiting-review and removed S-needs-code-changes labels

aneeshusa reviewed

buildbot/master/files/config/master.cfg Outdated

  
            MailNotifier(

                fromaddr="buildbot@example.org",

                sendToInterestedUsers=False,

                mode=["failing","exception"],

aneeshusa Feb 8, 2017

Member

style nit: needs a space after the comma after "failing"

Member

aneeshusa commented Feb 8, 2017

Changes so far LGTM, but I'll hand this review off since I don't know what SMTP settings would be appropriate here. r? @edunham

highfive assigned edunham and unassigned aneeshusa


        Adding space to mode list

c0ebb06

aneeshusa mentioned this pull request

Send notification of failed nightly builds #413

Open

Contributor

bors-servo commented Mar 15, 2017

☔ The latest upstream changes (presumably #620) made this pull request unmergeable. Please resolve the merge conflicts.

highfive added the S-needs-rebase label

Member

jdm commented Mar 15, 2017

@edunham Could you respond to the last comment please?

jdm closed this

jdm unassigned edunham

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

You can’t perform that action at this time.