You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/11/26 15:25:38 UTC
[GitHub] [guacamole-client] kevinimbrechts opened a new pull request #658: Adds SKIP_IF_UNAVAILABLE in options
kevinimbrechts opened a new pull request #658:
URL: https://github.com/apache/guacamole-client/pull/658
Adds properties to guacamole.process wich configure other options (like skip-if-unavailable)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #658: GUACAMOLE-1468: Adds SKIP_IF_UNAVAILABLE in options
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #658:
URL: https://github.com/apache/guacamole-client/pull/658#discussion_r757745100
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -710,6 +710,17 @@ associate_json() {
ln -s /opt/guacamole/json/guacamole-auth-*.jar "$GUACAMOLE_EXT"
}
+##
+## Adds properties to guacamole.process wich configure other
+## options
Review comment:
I think "other options" is a bit too vague here. It's unclear from reading the documentation for `associate_other()` exactly which configuration properties are in scope of the function. If the idea is that `associate_other()` covers any properties that are not tied to a specific extension, then I think that should be specified here.
Alternatively, there is already a section devoted to the few options that do not relate to an extension:
https://github.com/apache/guacamole-client/blob/c143c7cb5c73bcae51a4576c3bbe7d1d76ef7620/guacamole-docker/bin/start.sh#L794-L796
Simply placing the relevant call to `set_optional_property()` there may be better and simpler.
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -873,6 +884,9 @@ if [ -n "$JSON_SECRET_KEY" ]; then
associate_json
fi
+# Associate other keys
Review comment:
What is meant by "keys" here?
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -710,6 +710,17 @@ associate_json() {
ln -s /opt/guacamole/json/guacamole-auth-*.jar "$GUACAMOLE_EXT"
}
+##
+## Adds properties to guacamole.process wich configure other
Review comment:
Some typos here:
* "guacamole.properties"
* "which"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] necouchman commented on pull request #658: Adds SKIP_IF_UNAVAILABLE in options
Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #658:
URL: https://github.com/apache/guacamole-client/pull/658#issuecomment-980316819
Thanks, @kevinimbrechts. This will need to be associated with a Jira issue and tagged appropriately before we will merge it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] kevinimbrechts commented on pull request #658: GUACAMOLE-1468: Adds SKIP_IF_UNAVAILABLE in options
Posted by GitBox <gi...@apache.org>.
kevinimbrechts commented on pull request #658:
URL: https://github.com/apache/guacamole-client/pull/658#issuecomment-981431453
Hi @mike-jumper.
Thank you for the review.
I corrected typos and change du function name to associate_general_options. I added a better description.
Personally, I prefer to keep the function. Maybe placing guacd-hostname and guacd-port into associate_general_options is a good idea?
Let me know and I will adapt accordingly.
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] kevinimbrechts commented on pull request #658: GUACAMOLE-1468: Adds SKIP_IF_UNAVAILABLE in options
Posted by GitBox <gi...@apache.org>.
kevinimbrechts commented on pull request #658:
URL: https://github.com/apache/guacamole-client/pull/658#issuecomment-980468504
Hi @necouchman. Yes sorry. Is it ok now ? Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org