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
Feat update token seen at #2514
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 looks great IMO, just a small question but I might be overthinking it.
ed33001
to
59ee757
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.
This looks really good IMO
@@ -52,10 +53,16 @@ export class ApiTokenService { | |||
|
|||
private timer: NodeJS.Timeout; | |||
|
|||
private seenTimer: NodeJS.Timeout; |
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.
We could have made the precious timer in to an array, we only need to keep the reference to be able to clear them all.
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.
Hmm I'm not sure I understood this one, could you please clarify?
I guess they could both be inside a NodeJS.Timeout[]
but if we want a single reference to each we would store them in [0]
and [1]
I guess? 🤔 Besides one of them is an interval and the other a timeout.
Seems like decreased readability without much to gain, so I'm probably missing something.
Co-authored-by: Ivar Conradi Østhus <ivar@getunleash.ai>
Co-authored-by: Ivar Conradi Østhus <ivar@getunleash.ai>
https://linear.app/unleash/issue/2-448/update-last-seen-column-for-api-tokens