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 support for notification with server version greater than 9 #127

Open
wants to merge 13 commits into
base: oauth_setup
Choose a base branch
from

Conversation

Kshitij-Katiyar
Copy link

@Kshitij-Katiyar Kshitij-Katiyar commented Feb 4, 2025

Summary

Added support for notification with server version greater than 9

Screenshots

Screenshot from 2025-02-04 18-37-09

@Kshitij-Katiyar Kshitij-Katiyar added the 2: Dev Review Requires review by a core committer label Feb 4, 2025
@Kshitij-Katiyar Kshitij-Katiyar self-assigned this Feb 4, 2025
server/client_server.go Outdated Show resolved Hide resolved
server/flow.go Outdated Show resolved Hide resolved
server/user.go Outdated Show resolved Hide resolved
server/util/util.go Outdated Show resolved Hide resolved
@Kshitij-Katiyar
Copy link
Author

@wiggin77 Fixed the comments

var previousStartToken html.Token

for {
tt := domDoc.Next()
Copy link

@wiggin77 wiggin77 Feb 5, 2025

Choose a reason for hiding this comment

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

Since the only way out of this loop is when a html.ErrorToken is received, are we guaranteed to get this error at some point? There is no EOF token?

Copy link
Author

@Kshitij-Katiyar Kshitij-Katiyar Feb 7, 2025

Choose a reason for hiding this comment

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

Yes, html.ErrorToken is guaranteed.
html.ErrorToken is used to indicate an EOF

Copy link

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 One minor edit and a question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core committer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants