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.

Sign up for GitHub

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

Jump to bottom

refs #16294 - fixes wrong secure websocket detection #6115

Merged
merged 1 commit into from
Oct 7, 2018
Merged

refs #16294 - fixes wrong secure websocket detection #6115

merged 1 commit into from
Oct 7, 2018

Conversation

ohadlevy
Copy link
Member

@ohadlevy ohadlevy commented Oct 7, 2018

No description provided.

@theforeman-bot
Copy link
Member

Issues: #16294

@ohadlevy
Copy link
Member Author

ohadlevy commented Oct 7, 2018

missed this on #6056 /cc @UXabre.
@tbrisker if you have a minute for a quick merge, thanks!

ekohl
ekohl reviewed Oct 7, 2018
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Looking at this it might not be bad to store $('#vnc') in a variable or even generate the URL server side and store that in an attribute, but this in itself is correct.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.556% when pulling f88f1e6 on ohadlevy:ref-#16294 into a69735a on theforeman:develop.

tbrisker
tbrisker approved these changes Oct 7, 2018
View reviewed changes
Copy link
Member

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

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

makes sense, test failure unrelated

@tbrisker tbrisker merged commit fe86058 into theforeman:develop Oct 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl approved these changes

@tbrisker tbrisker tbrisker approved these changes

Assignees
No one assigned
Labels
Needs testing UI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ohadlevy @theforeman-bot @coveralls @ekohl @tbrisker