You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/11/25 17:46:28 UTC

[GitHub] [trafficcontrol] ocket8888 commented on pull request #6380: Upgrade github.com/lestrrat-go/jwx to 1.2.12

ocket8888 commented on pull request #6380:
URL: https://github.com/apache/trafficcontrol/pull/6380#issuecomment-979383801


   > Please note that I have only verified that `make unit` mostly works -- I did not run the `TestRemapConfig` and its related tests, and did not verify `trafficcontrol` running in any other form.
   
   Mostly we rely on automated testing these days, at least for changes like this. So don't worry about it, all the checks we want to run are being run.
   
   > Also, I reviewed your PR template, but didn't think this particular PR really fits into your regular type of PR. I especially have no clue how `trafficcontrol` works, and just changed the minimal code so that it works with the new version of jwx. And to that extent, I made the judgement call to set aside your templates, as I do not believe I can really respond to your PR template. I feel bad for ignoring the rules, but I hope you understand my position.
   
   That's fine. By the way, "`trafficcontrol`" sort of implies that this project is one main program, but actually it's more accurate to say that Traffic Control is a collection of software, rather than a single binary you run on its own. FYI.
   
   Anyway, as of right now we have three failing checks. First, it looks like the dependency on your old 0.9 version is still in there, so it needs a `go mod tidy`. Then, `go vet` is showing some things it would like changed. Idk if you want to bother with that though, because more importantly it seems like these changes break the API. It's actually pretty remarkable that you've managed to make those changes with no understanding of how ATC works and not break the stable APIs at all. But our unstable APIv4 needs client changes to support the server changes, and it's probably gonna need to be one of us that makes that change. I nominate @zrhoffman who first noticed our version of jwx was causing some issues in #6354.


-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org