On 12/8/2021 9:48 AM, Han-Wen Nienhuys wrote: > On Wed, Dec 8, 2021 at 3:35 PM Derrick Stolee <stolee@xxxxxxxxx> wrote: >> This is a HUGE diff, especially compared to the previous changes >> in this series. I recommend splitting this out into its own series >> and finding a way to break it down into smaller changes. > > Would you have a suggestion how? The reftable_record type is used > across the reftable library, so if we change its definition, that > impacts most callsites. Looking at the diff, I'm seeing a lot of things that don't seem necessary to changing the struct, like renaming "reftable_new_record" to "reftable_record_for". Yes, the implementation changes, but that implementation is related to the struct changing and can be done in a more isolated way. Some things can be split out easier, such as the addition of "uint8_t type" into the struct and replacing all reftable_record_type() calls with <var>.type. The big one is definitely the addition of a union, but it could be done in a way that only changes the implementation of methods with names such as reftable_record_from_obj() then we can replace those with "<var>.obj" later. Some of these changes are definitely going to have a lot of lines, but when it's the same kind of change it is easy to verify. I'm getting lost in this diff because it's doing too many things at once. Thanks, -Stolee
