Skip to content
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

fix: Make sure default status is set #18

Merged
merged 2 commits into from
Oct 3, 2019
Merged

fix: Make sure default status is set #18

merged 2 commits into from
Oct 3, 2019

Conversation

tkyi
Copy link
Member

@tkyi tkyi commented Oct 2, 2019

Context

Seeing some errors in API logs:

TypeError: Cannot read property 'includes' of undefined
at EmailNotifier._notify (/usr/src/app/node_modules/screwdriver-notifications-email/index.js:96:48)

Need to make sure build status object exists. Seems like some users are setting email settings like below, which is not handled correctly by this plugin:

settings:
    email:
        addresses: [[email protected], [email protected]]

Objective

This PR:

  • updates dependencies (jenkins-mocha, hapi, eslint, etc)
  • adds default statuses
  • pulls in hoek

License

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

Copy link
Member

@jithine jithine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need changes from users whose sd.yaml is currently valid ? is this backward compatible ?

@tkyi
Copy link
Member Author

tkyi commented Oct 3, 2019

@jithin1987 it shouldn't need changes unless they have empty array for addresses or statuses

@tkyi tkyi merged commit ec15dc3 into master Oct 3, 2019
@tkyi tkyi deleted the fixStatus branch October 3, 2019 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants