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

Revert #82 and #79 #109

Merged
merged 2 commits into from
May 10, 2023
Merged

Revert #82 and #79 #109

merged 2 commits into from
May 10, 2023

Conversation

@trilopin
Copy link

trilopin commented May 5, 2023

Is the missing functionality written anywhere? Can't that be completed without reverting the code?

@DHaussermann
Copy link

Thanks @mickmister I see this working as expected for the reverts.

As we are making a last set of changes here, Can you please add something to the read-me that covers an example of using the new guest-account support feature?
It would be helpful if you could just add that middle line below to the read-me as you already have changes in.

                        "DelayInSeconds": 3,
                        "IncludeGuests": false,
                        "Message": [
                        
                    

Copy link

@trilopin trilopin left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation @mickmister

@mickmister
Copy link
Contributor Author

As we are making a last set of changes here, Can you please add something to the read-me that covers an example of using the new guest-account support feature? It would be helpful if you could just add that middle line below to the read-me as you already have changes in.

@DHaussermann I'm inclined to do this in a separate PR as it's out of scope of reverting these two PRs. Thoughts on merging this PR as-is?

Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed as per comment above.

Read M edit will be addressed separately.

LGTM!

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels May 10, 2023
@mickmister mickmister merged commit f982137 into master May 10, 2023
@mickmister mickmister deleted the revert-82-and-79 branch May 10, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants