mike-jumper commented on a change in pull request #454: GUACAMOLE-793: CAS
Provider returns Group - like LDAP Provider
URL: https://github.com/apache/guacamole-client/pull/454#discussion_r370976737
##########
File path:
extensions/guacamole-auth-cas/src/main/java/org/apache/guacamole/auth/cas/ticket/TicketValidationService.java
##########
@@ -88,7 +92,7 @@
* If the ID ticket is not valid or guacamole.properties could
* not be parsed.
*/
- public Map<String, String> validateTicket(String ticket,
+ public TokensAndGroups validateTicket(String ticket,
Review comment:
> ...
>
> Certainly possible, but refactoring/restructuring someone elses code (and
then having to QA it) does seem like a different task and a bit more risk than
adding a feature to existing code.
>
> Also, does that really fit into the philosophy of "one jira ticket, one
change?" or is it "ok" to "fix" someone elses non-broken code as part of adding
a feature? ...
It does occasionally happen that additional code needs to be touched in
order to implement a feature. I don't think there's a universal "this way is
safe, this way is unsafe" answer to your question, but we do tend to look for
ways to improve established code when a new change indicates that it would be a
better direction. Doing otherwise would mean we trend toward greater and
greater technical debt.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
