<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">On 19.05.2016 10:58, Razvan Becheriu
      wrote:<br>
    </div>
    <blockquote
cite="mid:CAPj=nFtMdqfS0Q0Xc5T813BB3Tqzf=6sf8W1_1bPqM_nDVFc1w@mail.gmail.com"
      type="cite">
      <div dir="ltr">Please let me know how to proceed.</div>
    </blockquote>
    Hi Razvan,<br>
    I finally managed to complete this cycle of the review and posted my
    comments on github. The code seems to be mostly working. There are
    issues, but nothing really major that would prevent us from
    continuing the work on this. Everything I found is fixable. And I'm
    very, very impressed with the amount of work you put into
    unit-tests. I don't say that lightly. I think this is the best
    unit-tested patch ISC has ever received. So my overall impression is
    that you did a great work! Thank you very much for this.<br>
    <br>
    I'm not sure what's your previous experience with code reviews is.
    At ISC it is common for the review to be very detailed. Some may
    even say nit-picking. That's why we often try to split our tasks
    into smaller tickets. But that sometimes is not possible, like with
    the Cassandra code here. So I went over all my comments and listed 5
    that I consider the most essential. Just to recap, these are:<br>
    <ol>
      <li>
        rename dscsql to cass or cassandra (this is important, because
        it would eliminate nasty conflicts if we delay this rename)</li>
      <li>
        move TERASTREAM specific code to separate file or files (we
        can't have disabled code or customer specific code in out public
        repo. Also, moving the code to separate files will make long
        term maintenance a lot easier for you).<br>
      </li>
      <li>
        solve the client-id issue in unit-tests (this looks like an
        issue in the LeaseMgr. There's marginal chance that the issue is
        with tests, but it's very unlikely, because those tests are
        passing for 3 other backends).</li>
      <li>
        solve the preferred-lifetime issue in unit-tests</li>
      <li>
        write at least 2 missing unit-tests (for init and version) for
        kea-admin. These are essential, because every user in
        Kea+Cassandra will use them.<br>
      </li>
    </ol>
    <p>Once those are addressed, I think we would be able to merge the
      code to Kea master. Obviously, this will be far from being done
      and much more work will be needed, but we can do that in separate
      tickets/branches/pull requests. Also, ISC engineers will help with
      some of the tasks. Our timeline for Kea 1.1 is very tight, but we
      will do what we can. For a superb feature like this we can
      probably bend some schedules. :)<br>
    </p>
    <p>I have pushed couple minor fixes (and soft-wipe functionality
      which improves test execution time significantly) to github21
      branch.<br>
    </p>
    <p>Thanks a lot for your work on this.<br>
    </p>
    <p>Tomek<br>
    </p>
  </body>
</html>