On Tue, Mar 28, 2023 at 06:40:03PM +0200, Tomas Vondra wrote: > On 3/28/23 18:07, gkokola...@pm.me wrote: > > ------- Original Message ------- > > On Friday, March 24th, 2023 at 10:30 AM, gkokola...@pm.me > > <gkokola...@pm.me> wrote: > > > >> ------- Original Message ------- > >> On Thursday, March 23rd, 2023 at 6:10 PM, Tomas Vondra > >> tomas.von...@enterprisedb.com wrote: > >> > >>> This leaves the empty-data issue (which we have a fix for) and the > >>> switch to LZ4F. And then the zstd part. > >> > >> Please expect promptly a patch for the switch to frames. > > > > Please find the expected patch attached. Note that the bulk of the > > patch is code unification, variable renaming to something more > > appropriate, and comment addition. These are changes that are not > > strictly necessary to switch to LZ4F. I do believe that are > > essential for code hygiene after the switch and they do belong > > on the same commit. > > Thanks! > > I agree the renames & cleanup are appropriate - it'd be silly to stick > to misleading naming etc. Would it make sense to split the patch into > two, to separate the renames and the switch to lz4f? > That'd make it the changes necessary for lz4f switch clearer.
I don't think so. Did you mean separate commits only for review ? The patch is pretty readable - the File API has just some renames, and the compressor API is what's being replaced, which isn't going to be any more clear. @Georgeos: did you consider using a C union in LZ4State, to separate the parts used by the different APIs ? -- Justin
