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

Parametrize threshold #693

Merged

Conversation

mikerkelly
Copy link
Contributor

Allow the Slack request to specify the threshold rather than hard-coding it. We
may fine-tune this depending on how much output we get and what proves most
useful to users. This makes it easier to do that experimentation, change Slack
reminders etc.

I'll update the playbook with the current threshold and rationale. That'll be a
separate PR as it's a separate repo (team-manual).

As discussed in Slack:
https://bennettoxford.slack.com/archives/C069SADHP1Q/p1736434022622129?thread_ts=1736427127.238419&cid=C069SADHP1Q

Allow the Slack request to specify the threshold rather than hard-coding it. We
may fine-tune this depending on how much output we get and what proves most
useful to users. This makes it easier to do that experimentation, change Slack
reminders etc.

I'll update the playbook with the current threshold and rationale. That'll be a
separate PR as it's a separate repo (team-manual).

As discussed in Slack:
https://bennettoxford.slack.com/archives/C069SADHP1Q/p1736434022622129?thread_ts=1736427127.238419&cid=C069SADHP1Q
It wasn't as clear and useful as it could be. User and repo are most important
for working out if it needs responding to, so put them first. Include
"Deletion: " text before the datetime to make it clear to what it refers.

As discussed in Slack: https://bennettoxford.slack.com/archives/C069SADHP1Q/p1736765655458669?thread_ts=1736427127.238419&cid=C069SADHP1Q
Copy link
Contributor

@Jongmassey Jongmassey left a comment

Choose a reason for hiding this comment

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

LGTM

Am I right in thinking there are no tests for any of this workspace?

@mikerkelly
Copy link
Contributor Author

Thanks for the review, Jon.

Am I right in thinking there are no tests for any of this workspace?

That's correct. I don't think Iain had time as he was originally implementing it before moving team, and I didn't think it seemed like a big enough change to be worth writing tests. If the workspace needed more significant work it could be worth writing some, but I'll leave that to someone to decide when they come to do such work.

I don't think it's worth an Issue in isolation. Let me know if you disagree.

@mikerkelly mikerkelly merged commit b5f42c6 into main Jan 13, 2025
5 checks passed
@mikerkelly mikerkelly deleted the mikerkelly/workspace/codespaces/parametrize-threshold branch January 13, 2025 17:08
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.

2 participants