Migrate bsa, dns, batch, and reporting packages to java.time

[CydeWeys]

This change is 

[gbrodman]

@gbrodman reviewed 184 files and all commit messages, and made 11 comments.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on CydeWeys).

---

core/src/main/java/google/registry/rde/RdeUtils.java line 41 at r3 (raw file):

  private static final int PEEK_SIZE = 2048;

  public static final DateTimeFormatter RDE_WATERMARK_FORMATTER =

this is used in more than just RDE, maybe consider moving it to a more generic class?

---

core/src/main/java/google/registry/rde/RydeEncoder.java line 144 at r3 (raw file):

    /** Returns the built {@link RydeEncoder}. */
    public RydeEncoder build() throws IOException {

why are these exceptions added?

---

core/src/main/java/google/registry/rde/RydeTar.java line 47 at r3 (raw file):

  static ImprovedOutputStream openTarWriter(
      @WillNotClose OutputStream os, long expectedSize, String filename, Instant modified)
      throws IOException {

why did it make these changes?

---

core/src/test/java/google/registry/batch/BulkDomainTransferActionTest.java line 82 at r3 (raw file):

        persistResource(
            persistDomainWithDependentResources(
                    "pendingdelete", "tld", now, minusDays(now, 1), plusDays(now, 30))

why are you switching the plusMonths to plusDays in this test?

[CydeWeys]

@CydeWeys made 5 comments and resolved 1 discussion.
Reviewable status: 172 of 184 files reviewed, 11 unresolved discussions (waiting on gbrodman).

---

core/src/main/java/google/registry/rde/RdeUtils.java line 41 at r3 (raw file):

Previously, gbrodman wrote…

this is used in more than just RDE, maybe consider moving it to a more generic class?

Done, not sure it's a big improvement though.

---

core/src/main/java/google/registry/rde/RydeEncoder.java line 144 at r3 (raw file):

Previously, gbrodman wrote…

why are these exceptions added?

Removed.

---

core/src/main/java/google/registry/rde/RydeTar.java line 47 at r3 (raw file):

Previously, gbrodman wrote…

why did it make these changes?

Removed.

[gbrodman]

@gbrodman reviewed 12 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on CydeWeys).

[CydeWeys]

@CydeWeys made 1 comment and resolved 4 discussions.
Reviewable status: 168 of 195 files reviewed, 1 unresolved discussion (waiting on gbrodman).

---

core/src/test/java/google/registry/batch/BulkDomainTransferActionTest.java line 82 at r3 (raw file):

Previously, gbrodman wrote…

why are you switching the plusMonths to plusDays in this test?

Done.

[gbrodman]

@gbrodman reviewed 27 files and all commit messages, made 4 comments, and resolved 1 discussion.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on CydeWeys).

---

console-webapp/package-lock.json line 631 at r6 (raw file):

      }
    },
    "node_modules/@angular/build/node_modules/@types/node": {

the ai is not listening to you when you tell it to not modify this

---

core/src/main/java/google/registry/rde/RydeEncoder.java line 83 at r6 (raw file):

    } catch (Throwable e) {
      try {
        closer.close();

this shouldn't be in the constructor.......

---

core/src/test/java/google/registry/model/common/CursorTest.java line 42 at r6 (raw file):

  @BeforeEach
  void setUp() {
    createTld("tld");

if you're adding this, you can remove it in the test methods that have it

---

core/src/test/java/google/registry/tools/ListCursorsCommandTest.java line 42 at r6 (raw file):

  @BeforeEach
  void beforeEach() {
    createTld("tld");

i don't think this is necessary

[CydeWeys]

@CydeWeys made 2 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on gbrodman).

---

console-webapp/package-lock.json line 631 at r6 (raw file):

Previously, gbrodman wrote…

the ai is not listening to you when you tell it to not modify this

I do wonder what the value of excluding this really is. If it's out of date (which it has been for awhile), it gets updated any time you run the full build, which I do manually a lot too. Not sure how worth it it is to continuously have to remember to exclude changes to this file (either for me, or the AI).

---

core/src/main/java/google/registry/rde/RydeEncoder.java line 83 at r6 (raw file):

Previously, gbrodman wrote…

this shouldn't be in the constructor.......

Why? It made these changes in response to an automated errorprone comment, about exceptions not being handled.

[gbrodman]

@gbrodman made 1 comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on CydeWeys).

---

core/src/main/java/google/registry/rde/RydeEncoder.java line 83 at r6 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

Why? It made these changes in response to an automated errorprone comment, about exceptions not being handled.

Think about it instead of letting the AI do it. Why would we close everything in the constructor of the object? We close it in, unsurprisingly, the close() method.

[gbrodman]

@gbrodman made 1 comment and resolved 1 discussion.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on CydeWeys).

---

console-webapp/package-lock.json line 631 at r6 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

I do wonder what the value of excluding this really is. If it's out of date (which it has been for awhile), it gets updated any time you run the full build, which I do manually a lot too. Not sure how worth it it is to continuously have to remember to exclude changes to this file (either for me, or the AI).

yeah idc

[CydeWeys]

@CydeWeys made 1 comment.
Reviewable status: 167 of 220 files reviewed, 3 unresolved discussions (waiting on gbrodman).

---

core/src/main/java/google/registry/rde/RydeEncoder.java line 83 at r6 (raw file):

Previously, gbrodman wrote…

Think about it instead of letting the AI do it. Why would we close everything in the constructor of the object? We close it in, unsurprisingly, the close() method.

Reverted these changes. They're not even wrong though -- it is cleaner to close it immediately in a catch block rather than relying on this more obscure Closer method to do so. And it would only be closing them when an unrecoverable error is thrown, so doing so in the constructor is fine.

[CydeWeys]

@CydeWeys made 1 comment.
Reviewable status: 167 of 220 files reviewed, 3 unresolved discussions (waiting on gbrodman).

---

core/src/test/java/google/registry/model/common/CursorTest.java line 42 at r6 (raw file):

Previously, gbrodman wrote…

if you're adding this, you can remove it in the test methods that have it

Done.

[gbrodman]

@gbrodman reviewed 53 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on CydeWeys).

---

core/src/main/java/google/registry/rde/RydeEncoder.java line 83 at r6 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

Reverted these changes. They're not even wrong though -- it is cleaner to close it immediately in a catch block rather than relying on this more obscure Closer method to do so. And it would only be closing them when an unrecoverable error is thrown, so doing so in the constructor is fine.

eh, we haven't been worrying about unchecked exceptions here before and because this implements closeable, everything should be surrounding this in a try-with-resources anyway

[gbrodman]

@gbrodman reviewed 4 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on CydeWeys).

[CydeWeys]

PTAL, had to change a few more files.

[gbrodman]

@gbrodman reviewed 2 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on CydeWeys).

[gbrodman]

@gbrodman reviewed 2 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on CydeWeys).

[gbrodman]

@gbrodman reviewed 2 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on CydeWeys).