Conversation
StrangeGirlMurph
left a comment
There was a problem hiding this comment.
Haven't looked at the code yet. Just used the 10min I had to comment on the first things that came up.
There was a problem hiding this comment.
Really not necessary. Just let the interface in the config file. Makes it way easier to quickly look it up and read the docs
There was a problem hiding this comment.
The doc strings (/** */) are there so the IDE can show the docs when hovering over the value. Just cleans up the config files so there is less confusion in case less experienced users try Jambo. And you can just jump to the interface with ctrl click anyway
There was a problem hiding this comment.
That's a VSCode thing... It should be just a scroll away. One shouldn't need a high end IDE to figure things like that out.
There was a problem hiding this comment.
Thats not just a VSC thing? Such doc strings are part of js/ts and I do believe that no "high end IDE" is needed to see them when hovering over it
There was a problem hiding this comment.
Not entirely part of JS, but an extension called jsdocs. VScode is not the only ide that supoorts it (Webstorm supoorts it). It's also a human readable format so anyone reading this file can see all this info, if their IDE doesn't pick it up.
There was a problem hiding this comment.
Yeah sure "That's a VSCode thing" just meant that it's an IDE thing.
It's also a human readable format so anyone reading this file can see all this info, if their IDE doesn't pick it up.
Yeah sure but that's exactly my point. People shouldn't have to scroll to the top see where its imported from and then go to that file just to look up something that trivial which really doesn't hurt to be in that same file.
There was a problem hiding this comment.
Alternatively we could just
export const config = {...} as const;
without having any interfaces anywhere
There was a problem hiding this comment.
I think having a separate interface is good. "Stops" people from just blindly messing up stuff.
Co-authored-by: Murphy Sünnenwold <git@murphy-in.space>
flloschy
left a comment
There was a problem hiding this comment.
Resolving again...
src/db.ts
Outdated
| /** id of the log */ | ||
| id: string; | ||
| /** When a log got logged */ | ||
| date: Date; |
There was a problem hiding this comment.
No? Why?
Just a date object, the serializer takes care of converting everything to iso date and the reserialize returns a new date object using the iso string stored in the db
| /** How long a User played a certain game | ||
| * (Or the other way around) | ||
| */ | ||
| playtime: number; |
There was a problem hiding this comment.
idk, I simply dont like duration
And ms is what a presence-update gives you anyway
Sso why convert to duration, then instantly sterilize to ms again just to desterilize again to perform a simple math or timestamp thing...
Im not seeing the benefits behind for changing it




Rework of the whole tracking system
Featuring new commands and way better performance
(and commenting whoaaaaaaaa)