-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add separate module for the api package #1741
Conversation
633eaaa
to
dad8d6d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an expert in go mod shenanigans, but this looks OK to me.
We should probably emphasize in the next release log to import the API only, but not the app module. |
Since our vendored modules are checked in, have you run Edit: We have one, it's |
Good catch. Actually: can we get rid of vendor? I really don't see the point (images are built without it, quay is down more than github is). |
See #1597 |
@@ -48,3 +49,5 @@ require ( | |||
gopkg.in/square/go-jose.v2 v2.4.1 | |||
sigs.k8s.io/testing_frameworks v0.1.2 | |||
) | |||
|
|||
replace github.com/dexidp/dex/api => ./api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary, what happens if this is removed?
Looks like with this, we will have to revendor (if we are keeping that) every time this changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is necessary. If it is removed, go mod will try to download it from GitHub.
I don't think the api changes that much, but vendoring is not used when building the final image, so no harm done if we don't revendor. :)
The official docker release for this release can be pulled from dexidp/dex:v2.25.0 **Features:** - Move the API package to a separate module (dexidp#1741, @sagikazarmark) - OAuth2 Device Authorization Grant (dexidp#1706, @justin-slowik) - Support username, email and groups claim in OIDC connector (dexidp#1634, @xtremerui) **Bugfixes:** - Add offline_access scope in microsoft connector, if required (dexidp#1441, @jimmythedog) - Allow the google connector to work without a service account (dexidp#1720, @candlerb) **Minor changes:** - Remove vendor (finally) (dexidp#1745, @sagikazarmark) - Fix the LDAP example (dexidp#1762, @heidemn-faro) - Relocate the example app (dexidp#1764, @sagikazarmark)
The official docker release for this release can be pulled from dexidp/dex:v2.25.0 **Features:** - Move the API package to a separate module (dexidp#1741, @sagikazarmark) - OAuth2 Device Authorization Grant (dexidp#1706, @justin-slowik) - Support username, email and groups claim in OIDC connector (dexidp#1634, @xtremerui) **Bugfixes:** - Add offline_access scope in microsoft connector, if required (dexidp#1441, @jimmythedog) - Allow the google connector to work without a service account (dexidp#1720, @candlerb) **Minor changes:** - Remove vendor (finally) (dexidp#1745, @sagikazarmark) - Fix the LDAP example (dexidp#1762, @heidemn-faro) - Relocate the example app (dexidp#1764, @sagikazarmark)
Fixes #1726
This is a first step, we need to need a v2.0.0 version after merging.