On Fri, Jun 27, 2014 at 12:55 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 26 June 2014 19:08, Stefan Fuhrmann <stefan.fuhrmann_at_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.
Received on 2014-06-30 12:19:38 CEST