<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>Looks good.</p>
    <p>I believe the test could be further automatized, by adding more
      logic to the enum. E.g. each constant could define in a string a
      snippet of code that is unique to that version (and maybe the BASE
      version would have an empty snippet). That way the sources could
      be generated programatically - no need to repeat various headers
      etc. Also, I believe once you go down this path, it could be even
      possible to have a single checksrc method which takes a couple of
      version constants, generate the sources for them using the
      templating strategy described above.</p>
    <p>E.g. it would be great if all the 'mutable' parts would be
      contained to the enum declaration - adding enum constants is
      rather easy, especially when the structure is clearly
      self-explanatory (as in this case).<br>
    </p>
    <p>Maurizio<br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 03/07/18 00:38, joe darcy wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:7d15d77c-95d8-7e9a-891f-05b3c4547760@oracle.com">
      <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
      <p>Hi Peter,<br>
      </p>
      <br>
      <div class="moz-cite-prefix">On 7/2/2018 7:36 AM, Peter Levart
        wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:90aa4ad4-af3e-1c47-8b77-5c4e79c7fe60@gmail.com">
        <meta http-equiv="Content-Type" content="text/html;
          charset=utf-8">
        Hi Joe,<br>
        <br>
        A suggestion for more compact code: the (ever growing) switch
        statement in <span class="new">enum SourceTarget.checksrc could
          be replaced with a field of type BiConsumer<Versions,
          String[]>, initialized in constructor. For each enum
          constant you then just pass a method reference to the chosen
          target method. The checksrc in </span><span class="new"><span
            class="new">SourceTarget could be an instance method.<br>
          </span></span></blockquote>
      <br>
      I had an intermediate version along those lines before sending the
      first one out for review. In the intermediate version, I stored in
      the method reference in a field and the enum client code used an
      accessor method for it code. The downside was a bit unpleasent to
      use since the vararg-ness isn't captured in the generic type. But
      if checksrc was a varags instance on the enum, then it could
      benefit from the varargs conversion. Let's see ... yes, the
      resulting test looks better and should be easier to maintain:<br>
      <br>
          <a class="moz-txt-link-freetext"
        href="http://cr.openjdk.java.net/%7Edarcy/8206085.2/"
        moz-do-not-send="true">http://cr.openjdk.java.net/~darcy/8206085.2/</a><br>
      <br>
      Thank you for the suggestion.<br>
      <br>
      In any case, the span of versions should reduced soon by the
      removal of -source/-target/--release 6 in JDK 12 [1], which is one
      reason I'm working on refactoring these tests now :-)<br>
      <br>
      Cheers,<br>
      <br>
      -Joe<br>
      <br>
      [1] <a class="moz-txt-link-freetext"
href="http://mail.openjdk.java.net/pipermail/jdk-dev/2018-May/001210.html"
        moz-do-not-send="true">http://mail.openjdk.java.net/pipermail/jdk-dev/2018-May/001210.html</a><br>
      <br>
      <blockquote type="cite"
        cite="mid:90aa4ad4-af3e-1c47-8b77-5c4e79c7fe60@gmail.com"><span
          class="new"><span class="new"> <br>
          </span>Peter<br>
          <br>
        </span>
        <div class="moz-cite-prefix">On 06/29/2018 09:41 PM, joe darcy
          wrote:<br>
        </div>
        <blockquote type="cite"
          cite="mid:7c73f7f9-ab06-f734-633a-35173a2d45a9@oracle.com">Dis-charged
          revision along with JDK 11 specific source example now up at <br>
          <br>
              <a class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/%7Edarcy/8206085.1/"
            moz-do-not-send="true">http://cr.openjdk.java.net/~darcy/8206085.1/</a>
          <br>
          <br>
          Thanks, <br>
          <br>
          -Joe <br>
          <br>
          <br>
          On 6/29/2018 9:55 AM, Jonathan Gibbons wrote: <br>
          <blockquote type="cite">There's more static in this file than
            on my high-school Van de Graaf generator! <br>
            <br>
            Can we follow the convention of creating an instance in
            main, and then using instance methods. <br>
            <br>
            The naming of some methods is also novel, with respect to
            case-conventions. <br>
            <br>
            -- Jon <br>
            <br>
            <br>
            On 6/28/18 8:15 PM, joe darcy wrote: <br>
            <blockquote type="cite">Hello, <br>
              <br>
              Fresh off of updating <br>
              <br>
                  langtools/tools/javac/versions/Versions.java <br>
              <br>
              for the JDK 11 -> 12 transition, I'd like to refactor
              the test to reduce the maintenance needed when adding new
              versions (as in new releases) or removing versions (as is
              planned for later in JDK 12): <br>
              <br>
                  <a class="moz-txt-link-freetext"
                href="http://cr.openjdk.java.net/%7Edarcy/8206085.0/"
                moz-do-not-send="true">http://cr.openjdk.java.net/~darcy/8206085.0/</a>
              <br>
              <br>
              Thanks, <br>
              <br>
              -Joe <br>
              <br>
            </blockquote>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>