-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add a dry run flag to ensure we don't accidentally send out invites when testing #209
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little worried about checking in code that disables the invite logic, if we can figure out a flag instead that would make this more valuable.
src/invites.ts
Outdated
@@ -80,7 +80,8 @@ export async function resolveIdentifiers(client: ConferenceMatrixClient, people: | |||
|
|||
export async function invitePersonToRoom(client: ConferenceMatrixClient, resolvedPerson: ResolvedPersonIdentifier, roomId: string): Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can slip in a dry-run flag in the config, and just skip this function if so? Worst case, maybe we just override the inviteUser function in ConferenceMatrixClient
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A dry run flag is a much better approach - I've added one, let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. I've suggested we log that we log dry run when dry running a command to prevent confusion (esp if we toggle it on/off).
Feel free to merge when you're happy!
Co-authored-by: Will Hunt <[email protected]>
Merged #209 into main. |
Adds a flag to seal off sending live emails/invites, making testing less risky.