Skip to content
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

Fix typo in jwt package #1070

Merged
merged 5 commits into from Jun 19, 2021
Merged

Fix typo in jwt package #1070

merged 5 commits into from Jun 19, 2021

Conversation

amidam
Copy link
Contributor

@amidam amidam commented Mar 19, 2021

* change JWTTokenContextKey to JWTContextKey

* revise errors as well as comments

    * change JWTTokenContextKey to JWTContextKey

    * revise errors as well as comments
@amidam
Copy link
Contributor Author

amidam commented Mar 19, 2021

fixes #1071

auth/jwt/middleware.go Outdated Show resolved Hide resolved
	* revert the API identifier to its previous value

        * add JWTTokenContextKey side by side of JWTContextKey for historical compatibility
@peterbourgon
Copy link
Member

Can you please leave the old identifiers in place, and mark them as deprecated?

Amid added 2 commits June 2, 2021 21:54
	* define JWTContextKey as a new constant

        * mark JWTTokenContextKey as a deprecated constant

        * revise corresponding error messages
# Conflicts:
#	auth/jwt/middleware.go
@amidam
Copy link
Contributor Author

amidam commented Jun 2, 2021

Can you please leave the old identifiers in place, and mark them as deprecated?

I have done it already. I added a new constant called JWTContextKey and marked JWTTokenContextKey as a deprecated constant.

@amidam
Copy link
Contributor Author

amidam commented Jun 2, 2021

Can you please leave the old identifiers in place, and mark them as deprecated?

I have done it already. I added a new constant called JWTContextKey and marked JWTTokenContextKey as a deprecated constant.

Still they both have the same value

Copy link
Member

@peterbourgon peterbourgon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor thing then 👍

auth/jwt/middleware.go Outdated Show resolved Hide resolved
	* define JWTContextKey as a new constant

        * mark JWTTokenContextKey as a deprecated constant

        * revise corresponding error messages
@amidam
Copy link
Contributor Author

amidam commented Jun 2, 2021

One minor thing then +1

I have done it.

@amidam amidam requested a review from peterbourgon June 2, 2021 22:36
Copy link
Contributor

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sagikazarmark sagikazarmark mentioned this pull request Jun 19, 2021
@peterbourgon peterbourgon merged commit a5c7103 into go-kit:master Jun 19, 2021
@sagikazarmark sagikazarmark added this to the v0.11.0 milestone Jun 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants