On Fri, Jun 27, 2014 at 12:55 PM, Ivan Zhakov <i...@visualsvn.com> wrote:
> On 26 June 2014 19:08, Stefan Fuhrmann <stefan.fuhrm...@wandisco.com> > wrote: > > > > Hi Ivan, > > > > I see three alternative ways to code that function > > > > 1. As hard coded string / byte sequence (current implementation). > > Cons: > > * Hard to write, hard to review by just looking at it (applies to time > > until initial release only). > > Pros: > > * Explicitly coded as constant, deterring people from changing it. > > * Independent of other code, i.e. unintended changes to the format / > > encoding generated by the normal code usually become apparent > > when running the test suite. > > > > 2. Use our txn code to write r0. This should be simple and might > > at most require some special ID handling. > > Cons: > > * Generating incompatible r0s is likely not caught by our test suite > > (assuming that reader and writer functions are in sync). Basically > > all the risk that comes with dynamically generating a "constant". > > * Test cases must make sure our backward compat repo creation > > options create repos that can actually be used by old releases. > > (we might want explicit test for that anyway, though) > > Pros: > > * No or very little special code for r0 (not sure, yet). > > * Format / encoding changes don't require new r0 templates. > > > > 3. Write code to "stitch" r0 together, e.g. string_add(md5("END\n")). > > Cons: > > * No more robust than 1. but requires much more code. > > * May _look_ easy to understand but an actual offline review is still > hard. > > Pros: > > * Widely independent of other code, although not as much as 1. > > > > Do you generally agree with the range of options and their assessment? > Yes, I generally agree with range of options. The only other I have is > do not implement log addressing in first place. > > > Which one would you pick and why? > > > It's hard to pick option without looking to code, but I would start > with leaving string constant for revision body and then appending > indexes data using API. I.e. somewhat modified (2). > r1606554 generates the index data dynamically now. It makes repo creation slightly more expensive but that does not seem to affect our test suite in any significant way. So, I think that is not an issue. Are you o.k. with the code as it stands now? -- Stefan^2.