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

Added Schedules restrictions with command who-s-on-call #117

Merged
merged 3 commits into from
Nov 25, 2018
Merged

Added Schedules restrictions with command who-s-on-call #117

merged 3 commits into from
Nov 25, 2018

Conversation

eldios
Copy link
Contributor

@eldios eldios commented Nov 21, 2018

Added PagerDuty rescrition option (basic feature) to restrict the who's on call command.

Re: #109 (comment)

@eldios eldios changed the title Added Schedules restrictions with command Added Schedules restrictions with command who-s-on-call Nov 22, 2018
@stephenyeargin
Copy link
Member

We would want to update the README file to document the new setting.

@eldios
Copy link
Contributor Author

eldios commented Nov 22, 2018

@stephenyeargin README.md updated: 👍

renderSchedule = (s, cb) ->
withCurrentOncall msg, s, (username, schedule) ->
if (username)
messages.push("* #{username} is on call for #{schedule.name} - #{schedule.html_url}")
if !allowed_schedules or (allowed_schedules and schedule.id in allowed_schedules)
Copy link

@a-palchikov a-palchikov Nov 23, 2018

Choose a reason for hiding this comment

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

I think this could simpler:

if !allowed_schedules or schedule.id in allowed_schedules

Also, unless process.env.HUBOT_PAGERDUTY_SCHEDULES is always defined, reading it can yield an undefined value and split is only defined for strings (iirc), so I would rewrite this as:

allowed_schedules = pagerDutySchedules?.split(",")

Depending on whether it is desired to treat empty pagerDutySchedules as unspecified:

if !pagerDutySchedules?.length or schedule.id in allowed_schedules

which should probably be mentioned in the documentation as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@a-palchikov is that okay now?

@stephenyeargin stephenyeargin merged commit 42363f4 into hubot-archive:master Nov 25, 2018
@stephenyeargin
Copy link
Member

Released with v3.1.0

stephenyeargin added a commit that referenced this pull request Nov 27, 2018
Introduced in #117. Also fixes case where an ignored schedule would still appear in the logs if no username was assigned to it.

Fixes #118
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants