Skip to content

Conversation

@eguguchkin
Copy link
Contributor

Fixes #245


  • I have read and followed all requirements in CONTRIBUTING.md;
  • I used LLM/AI assistance to make this pull request;

@eguguchkin eguguchkin added this to the v0.62.5 milestone Nov 7, 2025
@eguguchkin eguguchkin requested review from cheb0, forshev and moflotas and removed request for moflotas November 7, 2025 11:51
for i := 0; i < fracCount; i++ {
appendDocs(t, actives[len(actives)-1], 500+rand.Intn(100))
actives = append(actives, fp.CreateActive())
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: some tests here make use of this pattern where docs are added to the last frac in actives slice and then new empty frac appended to actives, so fracs get filled the next iteration of a loop

maybe change it to more straightforward approach, which i find easier to understand, where new active frac created and filled with docs at the same time, like in TestReplayWithEmptyActive:

actives := make([]*frac.Active, 0, fracCount)
	for i := 0; i < fracCount; i++ {
		active := fp.CreateActive()
		appendDocs(t, active, 500+rand.Intn(100))
		actives = append(actives, active)
	}
	actives = append(actives, fp.CreateActive()) // last active frac is now empty

what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants