ocket8888 commented on pull request #5512: URL: https://github.com/apache/trafficcontrol/pull/5512#issuecomment-777677335
> I would probably hold off on this one until #5430 is completed/merged. That PR is actually needed for 5.1. Yeah, sure thing. > wouldn't this be a good PR to implement the TO Go client best practices discussed on the mailing list? I mean, I could. I don't have a problem with doing the work, I just thought at ~4.2k lines changed this PR was big enough. Basically this is the first step in accomplishing that, just cutting down the client methods to the ones that should exist and implementing query parameters one way to make the transition to RequestOptions easier. > wasn't sure if we really wanted to make all these breaking signature changes Because `traffic_ops/client` is deprecated/soon to be removed and people no longer import the same path for each, continually upgrading API client version we can absolutely just shatter the call signatures. It's not breaking anyone because they're explicitly import `traffic_ops/v3-client` which is totally unaffected. That's one of the real big advantages to having a separate client for each version. > But if we are, it might also be a good time to move resources into their own "modules" a la `Session.DeliveryServices.Get(...)`, `Session.Servers.Update(...)`? Maybe, "time" being a sort of flexible concept in this context. Between major revisions in an unreleased API version is a good "time", but that sounds like a huge amount of work that I wouldn't want to shove into this PR. I'm actually not entirely certain what that would look like code-wise and whether or not I'm sold on the idea. ---------------------------------------------------------------- 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]
