-
Notifications
You must be signed in to change notification settings - Fork 8
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
server.go and server_test.go refactor #1588
Conversation
Nazianzenov
commented
May 23, 2023
- response type is now id_token
- setting host and scheme for the URL contained in the QR Code (server.go)
* response type is now id_token * setting host and scheme for the URL contained in the QR Code (server.go)
@@ -37,7 +37,7 @@ const ( | |||
tokenLifeTimeHour = 24 | |||
|
|||
// qrSize size of QRCode SVG | |||
qrSize = 10 | |||
qrSize = 9 |
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.
this was change for aesthetic purpose (zooming too much with size 10 would make the QR Code way out of frame)
[PoP - Be2-Scala] Kudos, SonarCloud Quality Gate passed! |
[PoP - Be1-Go] Kudos, SonarCloud Quality Gate passed! |
[PoP - Fe2-Android] Kudos, SonarCloud Quality Gate passed! |
[PoP - Fe1-Web] Kudos, SonarCloud Quality Gate passed! |
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.
It's a small PR with clean / clear code. LGTM.
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.
Comment about a small detail, but LGTM for the rest
data := req.Host + req.URL.String() | ||
qrCode, err := qr.Encode(data, qr.M, qr.Auto) | ||
// add the scheme and host to the URL | ||
|
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.
Useless line break here