Ryan O'Hara-Reid

Results 66 comments of Ryan O'Hara-Reid

@gloriousCode I consolidated some code [here](https://github.com/thrasher-corp/gocryptotrader/pull/1578/commits/093e79472e85c21f0de0df874e99be39de4b874d) that you might need to look over since your last review.

@gbjk Havent done the file name changes yet waiting on review so that if your other PR gets merged first I can easily merge it then I will change it...

Yeah, I guess they are both valid so it's not a bug, but I would caution adding the difference though as you can get values above 100% in the negative...

> When you say "percentage difference" this doesn't feel like something you can average. The average is the point of reference rather than a percentage change from point A (as...

Flagged this as medium because this is beast. Sorry for those that cant use it. 🙏

> Makefile, GHA config and general functionality all looks good! Spotted this test error on it though: > > https://github.com/thrasher-corp/gocryptotrader/actions/runs/13253592337/job/36996357960#step:11:4906 [fixed](https://github.com/thrasher-corp/gocryptotrader/pull/1623/commits/a843b585c9feedc21450ef3277cfe080b3b0dedf) good spotting!

> This is a comment only about the overall ideaology > > I see from the tests that we are defaulting to sonic off, and we're testing amd64 with sonic...

> > Are you suggesting to default the main code to sonic out of the box not just the workflow tests? > > Yes Ok cool, > I propose that...

On testing upstream implementation this causes some issues when subscribing, can break out the subscription abstraction later but it's not currently needed and it's quite involved.

@samuael I added in your patch and did some adjustments to a type and the matching signature. Thanks heaps for looking into this.