[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #454: GUACAMOLE-793: CAS Provider returns Group - like LDAP Provider

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services