Conversation
|
To further this along, I've pulled this PR into build that I made at https://github.com/cardoe/dex/releases/tag/v2.44.90 You can grab the container to test with at ghcr.io/cardoe/dex:v2.44.90 and report back on this PR. |
|
@nabokihms we should pull this in |
|
@cardoe honestly, I'm not sure. Dex is not intended to be used as a library or be deployed as a separate binary. If we want to provide both, we have to add more tests, run tests for other OS etc. Having a replace makes it much harder to manage API package as a dependency. Makes releases harder. It seems like the initial request in this issue goes beyond just removing the replace. |
That's what my thinking was. Not for using it as a library. Just to simplify maintenance. |
I can certainly appreciate this, and to be honest with you this isn't the use case I had in mind for making this PR. Instead, as I mentioned in my original description for the PR, my goal with this change was to simply enable users to install Dex via the standard means ( I'm not asking or expecting the Dex project to provide or maintain pre-built artifacts for non-supported OS (in my case, macOS), and if I were to come across any issues specific to my use of Dex on macOS, I would certainly be happy to contribute fixes back to the project to address my own issues. Unfortunately as it stands, the I would encourage you to reconsider whether it's worth actively/knowingly prevent end-users wanting to use Dex in this way from doing so. |
|
You could rebase this as just a removal of the replace directive. |
Signed-off-by: Joonas Bergius <joonas@defenseunicorns.com>
c996739 to
09ae3b9
Compare
That's fair, it'd gone stale since I opened this. |
Overview
This PR gets rid of the replace directive in the
go.modfor thegithub.com/dexidp/dexmodule.Doing so has the benefit of making it possible to
go install github.com/dexidp/dex/cmd/dex, which at least slightly addresses the underlying need from #827, though an officially provided binary release would of course be better.What this PR does / why we need it
This change addresses the error message you'll receive if you try to
go install github.com/dexidp/dex/cmd/dex:It also bumps the
github.com/dexidp/dex/api/v2dependency tov2.4.0since parts of the code rely on types introduced in thegithub.com/dexidp/dex/api/v2releasev2.4.0.This has been previously brought up in a discussion.
Special notes for your reviewer