-
Notifications
You must be signed in to change notification settings - Fork 3
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
Route #13
Route #13
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.
What about the Github authentication? As far as I see this will be removed with this PR
src/components/OsdComponent.vue
Outdated
@@ -182,18 +182,31 @@ export default { | |||
// The users has selected an existing annotation | |||
// console.log('selected annotation') | |||
// console.log(annotation) | |||
console.log("anno is clicked") | |||
console.log("anno is clicked " , annotation) |
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.
are the changes to this file relevant?
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.
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.
more or less the same as above: the changes here are irrelevant(?) for the PR, so please remove them.
In 2e41245 you only removed one change but not the all
include "/GH_OAUTH_CLIENT.conf"; | ||
# reverse proxy to github access_token | ||
# set rule to /auth explicitly | ||
location = /auth { |
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.
Don't we need the GitHub authentication?
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.
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.
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.
Nothing changed here in 2e41245?!
Co-authored-by: Peter Stadler <[email protected]>
Co-authored-by: Peter Stadler <[email protected]>
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.
thanks for the updates but nginx.conf
still needs to be fixed, imho.
Add sub-path option for deployment