"Linus Arver via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > This patch series is the first 10 patches of a much larger series I've been > working. The main goal of this series is to enrich the API in trailer.h. The > larger series brings a number of additional code simplifications and > cleanups (exposing and fixing some bugs along the way), and builds on top of > this series. The goal of the larger series is to make the trailer interface > ready for unit testing. By "trailer API" I mean those functions exposed in > trailer.h. Are there places in the current code that deals with trailers but does not use the trailer API (e.g., manually parse and/or insert the trailer in an in-core buffer)? Is it part of the larger goal to update these places so that we will always use the trailer API to touch trailers, and if so, have these places been identified? Obviously the reason why I ask is that testing cannot be the goal by itself. The "alternative" ... > As an alternative to this patch series, we could keep trailer.h intact and > decide to unit-test the existing "trailer_info_get()" function which does > most of the trailer parsing work. ... may allow you to "test", but it would make it more difficult in the future to revamp the trailer API, if it is needed, in order to cover code paths that ought to be using but currently bypassing the trailer API. > This series breaks up "process_trailers()" into smaller pieces, exposing > many of the parts relevant to trailer-related processing in trailer.h. This > forces us to start writing unit tests for these now public functions, but > that is a good thing because those same unit tests should be easy to write > (due to their small(er) sizes), but also, because those unit tests will now > ensure some degree of stability across new versions of trailer.h (we will > start noticing when the behavior of any of these API functions change). And helper functions, each of which does one small thing well, may be more applicable to other code paths that are currently bypassing the API. > Thanks to the aggressive refactoring in this series, I've been able to > identify and fix a couple bugs in our existing implementation. Those fixes > build on top of this series but were not included here, in order to keep > this series small. It would be nicer to have a concise list of these fixes (in the form of "git shortlog") as a teaser here ;-). That would hopefully entice others into reviewing this part that forms the foundation. Thanks.
