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

ref: isTokenAboutToExpire for clarity and maintainability #2209

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

guipguia
Copy link
Contributor

@guipguia guipguia commented Jul 28, 2024

Refactor token handling function for clarity and maintainability

  • Split complex logic in isTokenAboutToExpire into helper functions:
    • decodePayload: Decodes the base64 payload from the token.
    • getExpiryTime: Extracts and converts the expiry time from the payload.
  • Simplified isTokenAboutToExpire main function for readability.

Solve this issue: #1848

@joaquimrocha joaquimrocha requested a review from yolossn July 29, 2024 09:14
backend/cmd/headlamp.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@illume illume left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks 🎉🎈

A nitpick about the commit message... For consistency it would be good if it could start with "backend:", so something like: "backend: istokenAboutToExpire: Refactor for clarify and maintainability".

@guipguia
Copy link
Contributor Author

guipguia commented Aug 6, 2024

Looks good to me. Thanks 🎉🎈

A nitpick about the commit message... For consistency it would be good if it could start with "backend:", so something like: "backend: istokenAboutToExpire: Refactor for clarify and maintainability".

Have updated the commit message to match the one suggested :)

@joaquimrocha joaquimrocha merged commit f288893 into headlamp-k8s:main Aug 8, 2024
5 checks passed
@joaquimrocha
Copy link
Collaborator

Obrigado @guipguia !

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.

4 participants