Guido Trotter

Results 9 comments of Guido Trotter

GC Benchmarks are a bit slow. Options: 1) Move them to a separate file with a -slow tag so that one can run them less often/on demand more easily 2)...

> So the file was validated on startup, but the notification still fails because the file was deleted after startup (could be weeks after startup). I think if we read...

> Looks ok, but I said that with the previous one and now I'm unsure :D > > I was wondering if the cleanupDone could be done with https://pkg.go.dev/sync#Once >...

> Don't know if there was/is actually a reason to not use t.TempDir(). But looks like an improvement to me Well it was added in 2020, and alertmanager is older....

LGTM, let's re-run the tests :)

> We are now failing this test which is vague and I remember debugging before but not documenting: > > https://github.com/prometheus/alertmanager/blob/aa879e10d1a254a51e8c6791c7fcec2b9fa530db/test/with_api_v2/acceptance/send_test.go#L422-L466 Maybe checking the hasFlushed condition would help this test...

This sounds like it may need some documentation changes as well, so people know it exists and to use it?

Could you run the benchmarks before and after, and then submit rather the output of benchstat, please? Otherwise it's a bit hard to compare...

The idea is to compare the benchmarks as ran without the patch applied, with the benchmarks ran after you applied it, to make sure the current use cases/code paths are...