Re: [PATCH 3/5] reftable/writer: simplify writing index records

From: Toon Claes <toon@iotcl.com>
Date: Jan 31, 2024, 13:44 UTC
Message-ID: <87sf2dppjt.fsf@iotcl.com>
In-Reply-To: <b0982baacf74a4501ce5c543b294bc15d6937875.1706263918.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> When finishing the current section we may end up writing index records
> for the section to the table. The logic to do so essentially copies what
> we already have in `writer_add_record()`, making this more complicated
> than it really has to be.

I didn't feel like this commit message made it easier for me to understand, because I interpreted words differently than you intended. Using "may end up" makes it sound like it's unexpected behavior. Also the use of "copies" implies to me it's doing a copy operation. I would rephrase it to something like:

  When finishing the current section some index records might be written
  for the section to the table. The logic to do so is essentially
  duplicated from what we already have in `writer_add_record()`, making
  this more complicated than it really has to be.

Other than that, I don't have any comments about this patch series.

-- Toon