Unit-test refactoring; removed some unused code too. #39

vdikov · 2025-04-26T21:49:31Z

Removed unused code from InputValidation.java
Fixed a bug: InvalidCanIdException was not keeping the bad canId, just a message about the out-of-band area
Replaced Theory based testing for Google Truth based one - Not a lot less code because of the introduction of a custom Subject, but Subjects is something we use elsewhere in the code-base at least, so people have more opportunities to see and learn that pattern
Replaced Hamcrest Matchers for Google Truth Matchers in ControUtilsTest.java

cuttestkittensrule

My opinion on the removal of theories in InputValidationTest is that it doesn't really improve the tests, and makes the tests themselves a bit more complicated. I do think it is a good idea to consistently use google truth, but we can we google truth in a test run with JUnit theories; all that JUnit theories allows us to do is more easily run the tests with many arguments.
While the removal of JUnit theories does mean we don't need the additional code in the theories directory to support it, I don't think that is a sufficient reason to remove them.

cuttestkittensrule · 2025-04-27T03:49:02Z

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

+  @Test
+  public void checkCanId_invalidCanIdTest() {
+    // Can IDs can only valid in the range [0, 62].
+    int invalidCanIds[] = {-50, -1, 63, 100};
+    for (int invalidCanId : invalidCanIds) {
+      var exception =
+          assertThrows(InvalidCanIdException.class, () -> InputValidation.checkCanId(invalidCanId));
+      assertThat(exception)
+          .hasCanId(invalidCanId)
+          .hasMessageThat()
+          .contains("is not a valid can id");
+    }
  }

-  @Theory
-  public void invalidCanID(
-      @Between(first = -50, last = -1) @Between(first = 63, last = 100) int canID) {
-    InvalidCanIdException expected = new InvalidCanIdException(canID);
-    InvalidCanIdException actual =
-        assertThrows(InvalidCanIdException.class, () -> InputValidation.checkCanId(canID));
-    assertEquals("Messages were not identical", expected, actual);
-  }
-
-  @Theory
-  public void genericInvalidTest(@Between(first = 0, last = 4) int unused) {
-    Integer min = Integer.valueOf(0);
-    Integer actual = Integer.valueOf(unused);
-    Integer max = Integer.valueOf(4);
-    InputValidation.checkBounds(min, max, actual, InputValidationTest::outOfBounds);
-  }
-
-  @Theory
-  public void validCanID(@Between(first = 0, last = 62) int canID) {
-    assertEquals(canID, InputValidation.checkCanId(canID));
+  @Test
+  public void checkCanId_validCanID() {
+    // Can IDs can only valid in the range [0, 62].
+    int validCanIds[] = {0, 1, 10, 62};
+    for (int validCanId : validCanIds) {
+      assertThat(InputValidation.checkCanId(validCanId)).isEqualTo(validCanId);
+    }
  }


Instead of prefixing with checkCanId, you should create a static nested class if you want to differentiate between different kinds of input validation.

Though at the moment, we only have one

Good idea. Done.

cuttestkittensrule · 2025-04-27T03:57:00Z

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

+    int invalidCanIds[] = {-50, -1, 63, 100};
+    for (int invalidCanId : invalidCanIds) {
+      var exception =
+          assertThrows(InvalidCanIdException.class, () -> InputValidation.checkCanId(invalidCanId));
+      assertThat(exception)
+          .hasCanId(invalidCanId)
+          .hasMessageThat()
+          .contains("is not a valid can id");
+    }


I like how you are checking the exception, though the new testing without theories is a lot less thorough. With the previous implementation, roughly 100 can ids are checked, and this only checks 4 ids on the bounds. The performance overhead isn't big for checking a large number of ids; on my machine it takes 0.015 seconds to run it all.

I'd say there is no point in being thorough in this test. The nature of this function is so trivial that we can reasonably expect there won't be any surprises mid-range (e.g., 55 suddenly is an invalid CanID, while -10 suddenly is a valid CanID).

The questions I'd rather want us to answer are:

Does this change improve readability? (when looked in isolation, this PR trades Theories for Subjects: hardly a readability win. But! if we establish that we stick with google Truth and thus with Subjects, we can reasonably expect that Prospect-Robotics maintainers should/will get accustomed to Subjects and we should avoid distracting them with other technologies in isolated tests)

Do these unit-tests help a random maintainer get more easily a grasp of what InputValidation.checkCanId does? Do we improve their "documentation" value? Do the unit-tests serve as good "example usage" snippets?

cuttestkittensrule · 2025-04-27T04:00:25Z

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

+  @Test
+  public void checkCanId_validCanID() {
+    // Can IDs can only valid in the range [0, 62].
+    int validCanIds[] = {0, 1, 10, 62};
+    for (int validCanId : validCanIds) {
+      assertThat(InputValidation.checkCanId(validCanId)).isEqualTo(validCanId);
+    }
  }


Same thing with other removal of Theories; this is a lot less thorough. With the previous test, the test would fail if any valid can id threw an exception

See my other comment. I don't think we need to thoroughly exhaust all possible valid/invalid values in this particular scenario.

cuttestkittensrule · 2025-04-27T04:01:42Z

lib/src/main/java/com/team2813/lib2813/util/InputValidation.java

-  private InputValidation() {
-    throw new AssertionError("non instantiable");
-  }


Probably don't want to make the constructor public by deleting it

Oops, this was an unintended change. Thanks for catching it!

cuttestkittensrule · 2025-04-27T04:07:51Z

lib/src/main/java/com/team2813/lib2813/util/InputValidation.java

@@ -52,6 +28,7 @@ static void checkBounds(
   * @return the {@code id}
   * @throws InvalidCanIdException if the id is invalid
   */
+  @Deprecated // Not used anywhere in org:Prospect-Robotics


If we think the utility functions for checking bounds are useful, then we may want to start using it (and/or other bound checking utilities in this class). In other words, is this Deprecated because it is bad, or is it bad that we aren't using it?

It was a speculative change on my end, aiming to provoke a discussion if we should deprecate and then remove this function if we are not going to use it.

Granted, this is a general utilities library and we can keep functions around even if we don't currently use them. I'm removing the Deprecated for now then.

This method wasn't public, so the removal-vs-deprecate argument appears to be moot.

Agreed that this appears to be speculative.

Un-deprecated it for now.

cuttestkittensrule · 2025-04-27T04:10:03Z

lib/src/test/java/com/team2813/lib2813/util/ControlUtilsTest.java

+    assertThat(ControlUtils.deadband(-1.0, 0.5)).isEqualTo(-1.0);
+    assertThat(ControlUtils.deadband(-0.75, 0.5)).isEqualTo(-0.5);
+    assertThat(ControlUtils.deadband(0.6, 0.5)).isWithin(1e-9).of(0.2);
+    assertThat(ControlUtils.deadband(0.75, 0.5)).isEqualTo(0.5);
+    assertThat(ControlUtils.deadband(1.0, 0.5)).isEqualTo(1.0);


Since actual computations are being done with these calls, we may want to use calls to isWithin for each of these. See google truth webpage

Yeah, things look more consistent if we apply isWithin everywhere. Done.

cuttestkittensrule · 2025-04-27T04:18:55Z

lib/build.gradle

@@ -67,13 +67,23 @@ tasks.named('jar') {
    }
 }

+// Generates Javadocs for implementation libraries under /build/docs/javadoc


This comment is a little bit misleading, since gradle will make the javadoc go in the specified directory regardless of the existence of the following code; the code that follows this comment just configures some extra javadoc tags.

Updated the documentation.

vdikov

Thanks for the review, @cuttestkittensrule !

Meta comment: here we have a good discussion around "Theories" vs "Truth" and we probably can get into quite a rabbit hole, each one proving their opinion is right :) On a high-level both versions are functionally ~equivalent, so the discussion boils down to, what I call, "matter of personal preference". And "matter of preferences" discussions are often the hardest to resolve.

As a code-owner and a SW lead you have the authority to continue upholding your opinion, your standards of quality, and your architecture philosophy and therefore you can continue requiring these to be met before you approve the PR. Or, and I'd like to implore you to consider this alternative for its broader effect on the team, you can let a developer who's just learning their ways in the code change things a bit more to their liking. I personally take the latter approach in my professional life because:

It allows incoming developers to build a sense of shared agency to the code.
It allows them to better familiarize themselves with the code and Java. There's hardly a better way to learn a code than when you are changing it - breaking it and fixing it in the process

One thing that's kind of ironical in this situation is that, if I were the code-owner, and you came around the next day proposing to implement a new unit-test with "Theories", I, following this same philosophy, will let you check it in. I'll then still come around a few months later and will propose: let's do another pass and see if we can simplify that code and remove the Theories. So, the end state is that we still don't allow Theories in the code, but we'd still benefit from doing this. With each round (adding Theories, then removing them) we learn something new and with each round we will have a discussion on whether or not the changes make sense.

vdikov · 2025-04-27T17:48:26Z

lib/build.gradle

@@ -67,13 +67,23 @@ tasks.named('jar') {
    }
 }

+// Generates Javadocs for implementation libraries under /build/docs/javadoc


Updated the documentation.

vdikov · 2025-04-27T17:48:59Z

lib/src/main/java/com/team2813/lib2813/util/InputValidation.java

-  private InputValidation() {
-    throw new AssertionError("non instantiable");
-  }


Oops, this was an unintended change. Thanks for catching it!

vdikov · 2025-04-27T17:51:41Z

lib/src/main/java/com/team2813/lib2813/util/InputValidation.java

@@ -52,6 +28,7 @@ static void checkBounds(
   * @return the {@code id}
   * @throws InvalidCanIdException if the id is invalid
   */
+  @Deprecated // Not used anywhere in org:Prospect-Robotics


It was a speculative change on my end, aiming to provoke a discussion if we should deprecate and then remove this function if we are not going to use it.

Granted, this is a general utilities library and we can keep functions around even if we don't currently use them. I'm removing the Deprecated for now then.

vdikov · 2025-04-27T17:53:07Z

lib/src/test/java/com/team2813/lib2813/util/ControlUtilsTest.java

+    assertThat(ControlUtils.deadband(-1.0, 0.5)).isEqualTo(-1.0);
+    assertThat(ControlUtils.deadband(-0.75, 0.5)).isEqualTo(-0.5);
+    assertThat(ControlUtils.deadband(0.6, 0.5)).isWithin(1e-9).of(0.2);
+    assertThat(ControlUtils.deadband(0.75, 0.5)).isEqualTo(0.5);
+    assertThat(ControlUtils.deadband(1.0, 0.5)).isEqualTo(1.0);


Yeah, things look more consistent if we apply isWithin everywhere. Done.

vdikov · 2025-04-27T18:48:44Z

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

+    int invalidCanIds[] = {-50, -1, 63, 100};
+    for (int invalidCanId : invalidCanIds) {
+      var exception =
+          assertThrows(InvalidCanIdException.class, () -> InputValidation.checkCanId(invalidCanId));
+      assertThat(exception)
+          .hasCanId(invalidCanId)
+          .hasMessageThat()
+          .contains("is not a valid can id");
+    }


I'd say there is no point in being thorough in this test. The nature of this function is so trivial that we can reasonably expect there won't be any surprises mid-range (e.g., 55 suddenly is an invalid CanID, while -10 suddenly is a valid CanID).

The questions I'd rather want us to answer are:

Does this change improve readability? (when looked in isolation, this PR trades Theories for Subjects: hardly a readability win. But! if we establish that we stick with google Truth and thus with Subjects, we can reasonably expect that Prospect-Robotics maintainers should/will get accustomed to Subjects and we should avoid distracting them with other technologies in isolated tests)

Do these unit-tests help a random maintainer get more easily a grasp of what InputValidation.checkCanId does? Do we improve their "documentation" value? Do the unit-tests serve as good "example usage" snippets?

vdikov · 2025-04-27T18:49:42Z

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

+  @Test
+  public void checkCanId_validCanID() {
+    // Can IDs can only valid in the range [0, 62].
+    int validCanIds[] = {0, 1, 10, 62};
+    for (int validCanId : validCanIds) {
+      assertThat(InputValidation.checkCanId(validCanId)).isEqualTo(validCanId);
+    }
  }


See my other comment. I don't think we need to thoroughly exhaust all possible valid/invalid values in this particular scenario.

vdikov · 2025-04-27T18:51:32Z

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

+  @Test
+  public void checkCanId_invalidCanIdTest() {
+    // Can IDs can only valid in the range [0, 62].
+    int invalidCanIds[] = {-50, -1, 63, 100};
+    for (int invalidCanId : invalidCanIds) {
+      var exception =
+          assertThrows(InvalidCanIdException.class, () -> InputValidation.checkCanId(invalidCanId));
+      assertThat(exception)
+          .hasCanId(invalidCanId)
+          .hasMessageThat()
+          .contains("is not a valid can id");
+    }
  }

-  @Theory
-  public void invalidCanID(
-      @Between(first = -50, last = -1) @Between(first = 63, last = 100) int canID) {
-    InvalidCanIdException expected = new InvalidCanIdException(canID);
-    InvalidCanIdException actual =
-        assertThrows(InvalidCanIdException.class, () -> InputValidation.checkCanId(canID));
-    assertEquals("Messages were not identical", expected, actual);
-  }
-
-  @Theory
-  public void genericInvalidTest(@Between(first = 0, last = 4) int unused) {
-    Integer min = Integer.valueOf(0);
-    Integer actual = Integer.valueOf(unused);
-    Integer max = Integer.valueOf(4);
-    InputValidation.checkBounds(min, max, actual, InputValidationTest::outOfBounds);
-  }
-
-  @Theory
-  public void validCanID(@Between(first = 0, last = 62) int canID) {
-    assertEquals(canID, InputValidation.checkCanId(canID));
+  @Test
+  public void checkCanId_validCanID() {
+    // Can IDs can only valid in the range [0, 62].
+    int validCanIds[] = {0, 1, 10, 62};
+    for (int validCanId : validCanIds) {
+      assertThat(InputValidation.checkCanId(validCanId)).isEqualTo(validCanId);
+    }
  }


Good idea. Done.

cuttestkittensrule · 2025-04-27T19:49:23Z

lib/src/test/java/com/team2813/lib2813/util/ControlUtilsTest.java

-    assertEquals(0.0, ControlUtils.deadband(-0.25, 0.5), 1e-9);
-    assertEquals(0.0, ControlUtils.deadband(0.0, 0.5), 1e-9);
-    assertEquals(0.0, ControlUtils.deadband(0.5, 0.5), 1e-9);
+    assertThat(ControlUtils.deadband(-0.5, 0.5)).isWithin(1e-9).of(0.0);


For this test method and the deadbandZwroDeadbandHasNoEffect, we actually don't want the isWithin call. To quote Google Truth's webpage,

Suitable uses for exact equality include cases where the contract of the code under test specifies…

…that it will copy values from the input to the output without doing any arithmetic on them.

…that it will return fixed values specified as exact double or float values, such as class constants or literals.

Suitable uses for approximate equality include cases where the contract of the code under test specifies…

…that it will do any kind of arithmetic.

…that it will return fixed values specified as mathematical values, such as 1/10 or π.

The contract for this method is that if the input is within the deadband, exactly 0.0 will be returned, so not using isWithin is appropriate for this test. For deadbandZwroDeadbandHasNoEffect, the contract of the tested method also states the input is returned as-is with a deadband of zero, so not using isWithin is appropriate. However, as per my prior comment, deadbandValuesOutsideDeadbandAreAdjusted tests the modification of the input when it is outside of the deadband, and should use isWithin because arithmetic is being performed.

Tldr; only deadbandValuesOutsideDeadbandAreAdjusted should use isWithin, because it is the only one of these three tests that tests a situation when the deadband method performs arithmetic.

Good plan. Done.

lib/src/test/java/com/team2813/lib2813/util/InvalidCanIdExceptionSubject.java

kcooney · 2025-04-28T04:50:17Z

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

+        assertThat(exception)
+            .hasCanId(invalidCanId)
+            .hasMessageThat()
+            .contains("is not a valid can id");


I think more straight-forward assertions would be more readable, and would remove the need for a custom Truth subject.

Also it might be good to verify that the invalid value is included in the exception message.

Suggested change

assertThat(exception)

.hasCanId(invalidCanId)

.hasMessageThat()

.contains("is not a valid can id");

assertThat(exception.getCanId()).isEqualTo(invalidCanId)

assertThat(exception)

.hasMessageThat()

.contains("not a valid can id");

assertThat(exception)

.hasMessageThat()

.contains(String.valueOf(invalidCanId));

Thanks, this is indeed so much better to do here.

I have to admit that I myself got carried away with the Subjects here - I just got curious what does it take to implement one :)

lib/src/main/java/com/team2813/lib2813/util/InvalidCanIdException.java

kcooney · 2025-04-28T04:54:09Z

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

+      // Can IDs can only valid in the range [0, 62].
+      int invalidCanIds[] = {-50, -1, 63, 100};
+      for (int invalidCanId : invalidCanIds) {
+        var exception =


nit: I think specifying the actual type here would allow readers that are not familiar with assertThrows() to have a better intuition of what is going on here.

Suggested change

var exception =

InvalidCanIdException exception =

My thinking here was that the type is already set by the first argument in assertThrows(InvalidCanIdException.class ...) here. So this is a situation where auto would be recommended in current C++ style guides. But that's not C++, and the IDE throws under-wrinkles this as an error, so I'm probably just wrong here.

kcooney · 2025-04-28T05:07:45Z

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

+      // Can IDs can only valid in the range [0, 62].
+      int validCanIds[] = {0, 1, 10, 62};
+      for (int validCanId : validCanIds) {
+        assertThat(InputValidation.checkCanId(validCanId)).isEqualTo(validCanId);


Here it's a bit hard to see what code is actually being tested. Introducing a local and adding a blank line would make it easier to visually see what is being tested (and would be closer to the Arrange-Act-Assert pattern):

Suggested change

assertThat(InputValidation.checkCanId(validCanId)).isEqualTo(validCanId);

int returnValue = InputValidation.checkCanId(validCanId);

assertThat(returnValue).isEqualTo(validCanId);

Note that if this were to fail, the test failure message wouldn't indicate which input value was provided (it would in the Theories-based version). You could do something like this:

Suggested change

assertThat(InputValidation.checkCanId(validCanId)).isEqualTo(validCanId);

int returnValue = InputValidation.checkCanId(validCanId);

assertWithMessage("%d should be considered a valid ID", validCanId)

.that(returnValue).isEqualTo(validCanId);

I personally find the Theories-based version of this test to be extremely easy to read, but concede that having to create a custom ParameterSupplier increases the cost of using Theories in this class. I'm not convinced that we should forbid use of Theories.

You're probably right. I'd suggest, let me get it my way this time just so I can gain a little more confidence with this code-base. I should not be allowed to do obviously wrong things in the code, but I hope it should be OK for me (or any other qualified contributor) to reshape it here and there to their liking ...

kcooney · 2025-04-28T05:12:37Z

lib/build.gradle

+    source = sourceSets.test.allJava
+    classpath = sourceSets.test.compileClasspath
+    destinationDir = file("${buildDir}/docs/testjavadoc")
+    println (title == null? '<null>' : title)


Suggested change

println (title == null? '<null>' : title)

println (title == null ? '<null>' : title)

kcooney · 2025-04-28T05:45:13Z

Thanks for the review, @cuttestkittensrule !

Meta comment: here we have a good discussion around "Theories" vs "Truth" and we probably can get into quite a rabbit hole, each one proving their opinion is right :)

As I mentioned on the Slack thread, Truth is an assertion framework; Theories is a runner (runners in JUnit 4 are the abstraction that runs tests). They have different goals.

Specially in this PR, you didn't replace Theories with Truth. You did two things:

replaced usages of Hamcrest/JUnit assertions to Truth
migrated a test class from the Theories runner to the JUnit4 runner (by introducing loops)

We can, of course, argue about the cost/benefits of Theories and Truth, but I find that framing it as "Theories" vs "Truth" to be confusing.

you can ... let a developer who's just learning their ways in the code change things a bit more to their liking

FWIW, in my professional life, I'm perfectly fine if people make the code easier to understand, reduce complexity, add clarifying comments, etc. I'm a bit hesitant to have people change between two equivalent coding styles (unless the previous style is uncommon or against best practices). My reasoning tends to be that at least two people already decided that the code was acceptable (the reviewer(s) and the original author) and changing code between styles doesn't add much value.

I think the larger concern is how many concepts a new member of the team will have to learn to be effective at making changes in the code. There's always something to learn, so we shouldn't be surprised if there are libraries and concepts that senior members of the team haven't used, of course. Mr. Dikov's concern appears to be about how much time it takes for someone to be reasonably effective at making changes to some/most of the code.

As I discovered when I started watching the Java training classes in the lab, there are a lot of concepts new people have to know just to understand Java. We don't (yet) teach WPILib, PathPlanner, use of AdvantageScope, Java testing or Git. We should certainly be asking ourselves whether the benefits of using the libraries we aren't required to use outweigh the learning curve for newer team members.

…onsistency across unit-tests

…st to use Google Truth matchers instead of Theories; fix a bug - InvalidCanIdException was not capturing a failing canId what looked like the intent of this class

vdikov · 2025-05-03T01:13:17Z

Thanks @kcooney and @cuttestkittensrule ! The PR is ready for another round. Please take another look!

lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java

vdikov changed the title ~~Refactor InputValidation and InputValidationTest.~~ Unit-test refactoring; removed some unused code too. Apr 27, 2025

vdikov requested a review from cuttestkittensrule April 27, 2025 03:07

cuttestkittensrule reviewed Apr 27, 2025

View reviewed changes

vdikov commented Apr 27, 2025

View reviewed changes

cuttestkittensrule reviewed Apr 27, 2025

View reviewed changes

kcooney reviewed Apr 28, 2025

View reviewed changes

vdikov and others added 4 commits May 3, 2025 00:19

Convert unit-test to use Truth instead of Hamcrest; aiming for more c…

85d002f

…onsistency across unit-tests

Remove unused methods form InputValidation; rewrite InputValidationTe…

1da1ea2

…st to use Google Truth matchers instead of Theories; fix a bug - InvalidCanIdException was not capturing a failing canId what looked like the intent of this class

Addressed PR reviewer's comments

d279663

Make canId final

7915771

vdikov force-pushed the vdikov/convert-tests-to-truth branch from fbce70c to 7915771 Compare May 3, 2025 00:23

vdikov added 3 commits May 3, 2025 00:51

Get rid of the InvalidCanIdExceptionSubject - it was an overkill

ac65145

Address more PR reviewer comments

a141695

Add an empty line at the end of the .gitignore file

426994e

Fixed broken assertWithMessage

c364882

vdikov requested review from kcooney and cuttestkittensrule May 4, 2025 01:56

kcooney reviewed May 4, 2025

View reviewed changes

Addressed Java style comments

7101c35

vdikov requested a review from kcooney May 4, 2025 16:42

kcooney approved these changes May 4, 2025

View reviewed changes

vdikov merged commit 2312a2b into main May 4, 2025
1 check passed

vdikov deleted the vdikov/convert-tests-to-truth branch May 4, 2025 21:07

-        assertThat(InputValidation.checkCanId(validCanId)).isEqualTo(validCanId);
+      int returnValue = InputValidation.checkCanId(validCanId);
+      assertThat(returnValue).isEqualTo(validCanId);

	println (title == null? '<null>' : title)
	println (title == null ? '<null>' : title)

Unit-test refactoring; removed some unused code too. #39

Unit-test refactoring; removed some unused code too. #39

Conversation

vdikov commented Apr 26, 2025 • edited Loading

cuttestkittensrule left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdikov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kcooney Apr 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kcooney commented Apr 28, 2025 • edited Loading

vdikov commented May 3, 2025

vdikov commented Apr 26, 2025 •

edited

Loading

kcooney Apr 28, 2025 •

edited

Loading

kcooney commented Apr 28, 2025 •

edited

Loading