Moving the consumer to typescript

[iPromKnight]

Wip.. Her for observability right now. Not much left to go.

[sleeyax]

Cool, would like to review this when I'm back online

[purple-emily]

Damn sleeyax! Nice

[iPromKnight]

Initial review, mostly questions and suggestions for improvement. Code style and personal preferences are to be discussed further with the team.

A few more things I would like to mention:

• There's an inconsistency in file naming. Some files are named in camelCaseand others in snake_case. It doesn't really matter which one we pick but it should be consistent.
• I'm not particularly fond of the 'services' approach; it seems like a concept directly borrowed from C# and applied to TypeScript :sweat_smile:

Yeah totally - I'm going through renaming them as I go - though ti'd caught that one I was thinking snake_case for file names, and pascalCase for imports? Thanks a lot - you see this is why i asked for you to review it, difference between a reader of a language and a writer of a language 😉

The services idea is kinda c# - i dunno what the naming convention would be in TS - provider? Or just torrent_entries etc Main concept is ensuring the single responsibility principle

[sleeyax]

I was thinking snake_case for file names, and pascalCase for imports?

Fine with me :)

The services idea is kinda c# - i dunno what the naming convention would be in TS - provider? Or just torrent_entries etc Main concept is ensuring the single responsibility principle

Hmm I did a sloppy job at the end there explaining my concern. There's no issue with the services themselves, or how they are named, but just the way they are used. Exporting instances of classes isn't common in JS/TS. I suggest looking into introducing a dependency injection layer like typedi. I'll try to dedicate some time to this tomorrow.

[iPromKnight]

I was thinking snake_case for file names, and pascalCase for imports?

Fine with me :)

The services idea is kinda c# - i dunno what the naming convention would be in TS - provider? Or just torrent_entries etc Main concept is ensuring the single responsibility principle

Hmm I did a sloppy job at the end there explaining my concern. There's no issue with the services themselves, or how they are named, but just the way they are used. Exporting instances of classes isn't common in JS/TS. I suggest looking into introducing a dependency injection layer like typedi. I'll try to dedicate some time to this tomorrow.

Absolutely agreed - felt meh when doing it Looking at the repo for typedi, its not maintained anymore and everyone else has moved over to inversify I'll add today

[sleeyax]

Ehw why switch to yarn all of a sudden?

This PR is becoming much more than just a typescript rewrite, sneaking in other changes isn't nice.

[iPromKnight]

It's a small change.

I can change it back. Parallel restores seem much faster with it is the only reason I switched.

Definitely not snuck in though, I made sure it was labelled in two pr updates lol

Everything else that had been tested (like move from esbuild, and torrent-stream) had already been rolled back. Saved me creating a new branch to poc what was in flight, all which is fine in draft

[iPromKnight]

Love the added unit tests, good work!

Thanks 😄 It definitely needed them

[purple-emily]

Wait it’s tagged as done!? Nice!! I’ll wait for @sleeyax to tell me what opinion I should have 😂

[iPromKnight]

Cheers. Aye one unresolved comment atm. Will see and then I'll rebase on main and resolve conflicts 😄

[sleeyax]

Wait it’s tagged as done!? Nice!! I’ll wait for @sleeyax to tell me what opinion I should have 😂

It's marked as ready for review. I've reviewed 2x, maybe let us know your own opinion by reviewing too? I'm not gonna tell you what to think 😂

[purple-emily]

@iPromKnight Anything else left to do? I'm not a competent JS/TS developer so I am not offering anything to this discussion :)

[iPromKnight]

@iPromKnight Anything else left to do? I'm not a competent JS/TS developer so I am not offering anything to this discussion :)

I dont think we have anyone else to review it I've looked through as much as I can to try to find anything missed, but its hard to review mistakes you've made yourself 😄

Its ready to merge I think

[sleeyax]

Alright LGTM from my end too, we can still improve things in another PR later down the line :) To who goes the honors of hitting the merge button?

[iPromKnight]

In the words of SLJ

[sleeyax]

:clinking_glasses: