-
Notifications
You must be signed in to change notification settings - Fork 108
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
Restore flexibility in keycloak-js dependency #3391
Conversation
With 21.1.0-1 the keycloak-masthead dependency has been fixed, so restore the circumflex to allow "21.*" behavior.
@@ -22,7 +22,7 @@ | |||
"gulp": "^4.0.2", | |||
"jest": "^27.5.1", | |||
"js-cookie": "^3.0.1", | |||
"keycloak-js": "21.0.2", | |||
"keycloak-js": "^21.0.0", |
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.
Since we know 21.0.2 at least worked, should we be setting that as the minimum?
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.
This is supposed to be taking the latest build of the major version. So, yeah; I could have updated 21.0.1 to 21.0.2, but we're really currently using 21.1.0-1, and soon something later, so that seemed pointless and even misleading. (Although there are some weird semantics here because 21.1.0
gives me the broken 21.1.0 instead of the fixed 21.1.0-1; and I gave up trying to figure out why.
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.
If the intention here is to roll this back to "what it was", then we should be using "^21.0.1"
.
If we want to roll forward, then it should probably be "^21.10.0"
or whatever the current version is today.
On the gripping hand, "^21.0.0"
is probably more than adequate, since I think we're going to pull the latest v21 regardless of what we put here.
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.
Restoring what was there, or specifying ^21.0.2
, seemed pointless as we don't expect to use 21.0.x; we expect to use the latest 21.x. Oddly, npm
didn't work when I tried 21.1.0
; it used exactly that (broken) version, despite the ^
which I thought ought to have avoided that. So I ended up going for the "generic" 21.0.0 "base version", which works as I expect and doesn't convey quite the same intent of a specific minor version. Really, the npm semantics for both ^
and ~
seem odd and I'm not sure either is really exactly what we ought to want; but the documentation I've found is squishy enough that I'm not entirely convinced of that, it works, it's broadly used through our package.json
, and I decided not to try alternatives.
@@ -22,7 +22,7 @@ | |||
"gulp": "^4.0.2", | |||
"jest": "^27.5.1", | |||
"js-cookie": "^3.0.1", | |||
"keycloak-js": "21.0.2", | |||
"keycloak-js": "^21.0.0", |
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.
If the intention here is to roll this back to "what it was", then we should be using "^21.0.1"
.
If we want to roll forward, then it should probably be "^21.10.0"
or whatever the current version is today.
On the gripping hand, "^21.0.0"
is probably more than adequate, since I think we're going to pull the latest v21 regardless of what we put here.
With 21.1.0-1 the keycloak-masthead dependency has been fixed, so restore the circumflex to allow "21.*" behavior.