Localization sweep #306

sirpengi · 2023-09-20T22:03:46Z

I've gone through the code and found any hard-coded strings or other TODO(i18n) markers and wired them up to the localization framework.

For some more complicated translations that didn't have inherit access to a BuildContext (exception messages, or certain widget labels and tooltips) I opted for a pattern of attaching methods that take the ZulipLocalizations object so they could perform the translation when needed.

gnprice · 2023-09-21T04:27:25Z

Thanks! I was preoccupied today with the Sentry-induced crashes in zulip-mobile that we resolved with v27.213, so I didn't get to a detailed review of this.

A couple of high-level comments which will make a good first pass:

Let's in general squash the change that adds a given string to the ARB file together with the change that starts consuming it in our code. That provides context when reading either part of the change, from seeing the complementary part of it there in the same commit.
Many of these changes are nice and easy and boring, and it's fine to have those squashed together with each other. (Though I'd also be happy to have them split more finely.) But it looks like some are more substantive and interesting — in particular, the changes in lib/api/.

It'd be good to have a separate commit for those lib/api/ changes, and probably one or more others for the other groups of "more complicated translations" that have the pattern you mention.

sirpengi · 2023-09-25T20:26:38Z

@gnprice the PR has been split and bunched as requested, ready for review

gnprice

Thanks! That split is very helpful.

Comments below. I read the lib/api/ commit fully, and skimmed through the rest. Several of the points below apply in several places across several commits; taken that way, I think they broadly cover all the feedback I'd have.

gnprice · 2023-09-26T01:52:05Z

lib/api/exception.dart

+  ///
+  /// For [ZulipApiException] this may be supplied by the server as the `message`
+  /// property in the JSON response.
+  // TODO(i18n): no method yet to translate a server provided string.


There's no TODO to be done here, happily; the server-provided string is already translated. See https://zulip.com/api/rest-error-handling .

gnprice · 2023-09-26T01:56:09Z

test/widgets/clipboard_test.dart

-                  // TODO(i18n)
                  copyWithPopup(context: context, successContent: const Text('Text copied'),


This line is deleted in a commit that doesn't seem otherwise related to this code.

gnprice · 2023-09-26T02:00:37Z

lib/api/exception.dart

+  /// For [ZulipApiException] this may be supplied by the server as the `message`
+  /// property in the JSON response.


Why the change from is to may be? Seems like it still is.

You're right, the may be applied to the broader ApiRequestConnection class, where in some cases the message was supplied by the server, but that is the case specifically of ZulipApiException

gnprice · 2023-09-26T02:01:49Z

lib/api/exception.dart

+  /// Server supplied user-facing description of the error.
+  ///
+  /// For [ZulipApiException] this may be supplied by the server as the `message`
+  /// property in the JSON response.
+  // TODO(i18n): no method yet to translate a server provided string.
+  final String? message;


As the meaning of this is changing to be more specific, it should get a more specific name to match. serverMessage would fit this description well.

gnprice · 2023-09-26T02:05:49Z

lib/api/exception.dart

+  String toTranslatedString(ZulipLocalizations zulipLocalizations) {
+    return (translatableMessage != null)
+      ? translatableMessage!(zulipLocalizations)
+      : super.toString();
+  }


I kind of think that this should become the only public way to read the error message, so that the two fields that back it would become private. It seems like in this design, it'd be a bug if we ever take the plain message and show that to the user, right? And that'd be a very easy bug to have.

In fact, asking the IDE to show references to the field, it seems like in this version of the PR we already introduce that bug — there's a reference to e.message in lib/widgets/login.dart:

final message = (e is ZulipApiException) ? zulipLocalizations.loginServerErrorMessage(e.toTranslatedString(zulipLocalizations)) : e.message; showErrorDialog(context: context, title: zulipLocalizations.loginServerLoginFailed, message: message);

Then the name of this method can also become something simpler, like message.

For this (and the previous comment) I've consolidated both into a message attribute that takes the localizations object (even in the case where we won't run the string through l10n, as when it's supplied by the server)

gnprice · 2023-09-26T02:10:02Z

lib/widgets/action_sheet.dart

-  String get label;
+  String Function(ZulipLocalizations) get translateLabel;


Separately, it doesn't seem like this benefits from being a getter, now that it's returning a function — it should just be a method, now taking an argument but still returning a String.

gnprice · 2023-09-26T02:11:41Z

assets/l10n/app_en.arb

+  "actionSheetShare": "Share",
+  "@actionSheetShare":  {
+    "description": "Label for share button on action sheet."


I like the "action sheet" prefix on these names. Perhaps a bit more explicitness to further scope what the context is about: actionSheetOptionShare.

gnprice · 2023-09-26T02:16:30Z

assets/l10n/app_en.arb

+  "actionSheetCouldNotFetchMessageSource": "Could not fetch message source",
+  "@actionSheetCouldNotFetchMessageSource": {
+    "description": "Dialog message when the source of a message could not be fetched."


For these error messages, I feel like the relevant context isn't so much "action sheet" as that it's the error message to appear in a dialog box.

As a test for making these naming choices, consider if we had some other place in the UI — somewhere other than an action sheet — where you could do something that needed to fetch the source of a message, and that fetching failed. Would we want the message to be different? I think we'd want it to be the same (and therefore save translators' effort re-translating it, and avoid showing users untranslated versions until they do).

So perhaps errorCouldNotFetchMessageSource. Potentially could be like errorDialogCouldNotFetchMessageSource, but I think the "dialog" specificity wouldn't be helpful either — if we show it in a toast/snackbar or something, we'll still want the same message.

gnprice · 2023-09-26T02:21:49Z

assets/l10n/app_en.arb

+  "dialogButtonOK": "OK",
+  "@dialogButtonOK": {
+    "description": "Button label in generic dialogs to confirm."


The actual use of this is somewhat more specific — it's in an error dialog, as the only available action, to acknowledge the error and dismiss the dialog.

I think that's a relevant distinction: it's fairly likely that if we had a dialog box with an actual choice, with options like perhaps "OK" / "Cancel", then in some languages the "OK" would be translated differently from the "OK" of the error dialog.

gnprice · 2023-09-26T02:23:30Z

assets/l10n/app_en.arb

+  "dialogButtonCancel": "Cancel",
+  "@dialogButtonCancel": {
+    "description": "Button label in generic dialogs to cancel."
+  },
+  "dialogButtonContinue": "Continue",
+  "@dialogButtonContinue": {
+    "description": "Button label in generic dialogs to continue."


Similarly these names and descriptions should be a bit more specific.

The function name is showSuggestedActionDialog. So e.g. "to continue with a suggested action" / "to cancel a suggested action".

sirpengi · 2023-09-26T15:49:31Z

@gnprice ready for review again. In addition to your comments, I've consolidated most generic errors into their own prefix if it felt like they could be reused.

gnprice

Thanks for the revision! Comments below.

Oh and a commit-message nit:
38498c4 msglist [nfc]: update i18n todo with specific issue
ea9c599 clipboard test [nfc]: Remove unnecessary TODO
3e20a17 core: Add translations for exceptions related to api requests
86753c1 action_sheet: translations
5edc7a8 dialog: translations
990548f lightbox: translations
d49c1a6 login: translations
629e84a compose: translations

Use sentence case in the summary lines, same as at #305 (comment) .

gnprice · 2023-09-26T23:19:26Z

lib/api/exception.dart

  /// For [ZulipApiException] this is supplied by the server as the `message`
  /// property in the JSON response, and is translated into the user's language.


Suggested change

/// For [ZulipApiException] this is supplied by the server as the `message`

/// property in the JSON response, and is translated into the user's language.

/// For [ZulipApiException] this is supplied by the server as the `message`

/// property in the JSON response.

The "is translated" part is now covered by the type signature.

gnprice · 2023-09-26T23:19:41Z

lib/api/exception.dart

  ApiRequestException({required this.routeName, required this.message});
+
 }


stray blank line

gnprice · 2023-09-26T23:23:50Z

assets/l10n/app_en.arb

+  "apiConnectionNetworkRequestFailed": "Network request failed",
+  "@apiConnectionNetworkRequestFailed": {
+    "description": "Message for error dialog when a network request fails."


Let's give this and the other two from the same commit the "error" prefix, like in some later commits — I think that works well. So like errorNetworkRequestFailed.

gnprice · 2023-09-26T23:25:38Z

assets/l10n/app_en.arb

+      "httpStatus": {
+        "type": "int",
+        "example": "500"
+      }


nit: Can we fit these placeholders on one line each? From browsing the examples so far, I think we can:

Suggested change

"httpStatus": {

"type": "int",

"example": "500"

}

"httpStatus": {"type": "int", "example": "500"}

That'd make it a bit easier to scan through these.

gnprice · 2023-09-26T23:29:22Z

assets/l10n/app_en.arb

+  "@composeBoxAttachFromCameraTooltip": {
+    "description": "Tooltip for compose box icon to attach an image from the camera to the message."
+  },
+  "composeBoxSelfDMHint": "Jot down something",


Suggested change

"composeBoxSelfDMHint": "Jot down something",

"composeBoxSelfDmHint": "Jot down something",

and similarly for the next few below.

See the definition of camel case linked from the Flutter style guide.

gnprice · 2023-09-27T00:37:55Z

assets/l10n/app_en.arb

+  "loginValidationPassword": "Please enter your password",
+  "@loginValidationPassword": {
+    "description": "Prompt for input for password field."


I'd describe this as stronger than a prompt — it's an error message.

In particular it gets returned from a validator callback, which is documented (at [FormField.validator]) as:

/// An optional method that validates an input. Returns an error string to /// display if the input is invalid, or null otherwise. /// /// The returned value is exposed by the [FormFieldState.errorText] property. /// The [TextFormField] uses this to override the [InputDecoration.errorText] /// value.

So the returned value is "an error string", exposed by an errorText property.

This specific error message is for when the user didn't enter a password in the password field. So could be loginErrorMissingPassword.

gnprice · 2023-09-27T00:41:55Z

assets/l10n/app_en.arb

+  "permissionsNeededCameraAccessDenied": "To upload an image, please grant Zulip additional permissions in Settings.",
+  "@permissionsNeededCameraAccessDenied": {
    "description": "Message for dialog when the user needs to grant permissions for camera access."
  },
+  "permissionsReadExternalStorageDenied": "To upload files, please grant Zulip additional permissions in Settings.",
+  "@permissionsReadExternalStorageDenied": {


These should be consistent about whether "needed" is a part of their naming prefix.

The "needed" does feel kind of redundant with "denied". Perhaps permissionDeniedFooBar?

gnprice · 2023-09-27T00:44:09Z

assets/l10n/app_en.arb

+  "permissionsNeededError": "Error",
+  "@permissionsNeededError": {
+    "description": "General title for access dialog when we encounter an unknown error."


This doesn't seem to get used.

There were some unreplaced strings this was meant to stand in for, so this is being used now.

gnprice · 2023-09-27T00:46:01Z

assets/l10n/app_en.arb

+      "num": {
+        "type": "int",
+        "example": "4"
+      },
+      "maxFileUploadSizeMib": {
+        "type": "int",
+        "example": "15"
+      },
+      "listMessage": {
+        "type": "String",
+        "example": "foo.txt\nbar.txt"


nit: make the example data consistent — if it's two filenames, then the number should be 2

gnprice · 2023-09-27T00:49:06Z

lib/widgets/compose_box.dart

-class _AttachFileButton extends _AttachUploadsButton {
-  const _AttachFileButton({required super.contentController, required super.contentFocusNode});
+class _AttachFileButton extends _AttachUploadsButton { const _AttachFileButton({required super.contentController, required super.contentFocusNode});


This seems unintended 🙂

sirpengi · 2023-09-29T21:51:31Z

@gnprice ready for another go!

gnprice

Thanks for the revision! This is definitely converging — I think at this point I'm happy with all the structure of the code here.

Comments below. Several of them are places where I spotted a discrepancy between the existing hard-coded string and the new English version. It should be doable to really have zero of those, by rereading through the draft changes; I'd like to get to that point before merge, because that's otherwise a potential way for typos and other errors to sneak into the UI.

Related to that is ways of keeping down the complexity of reviewing these changes. One thing that's contributed to making these changes as readable as they are is how you've generally kept the strings grouped thematically, and ordered within each group either to match the corresponding code or alphabetically; that's been very helpful.

I think the biggest other area that would help there is splitting up the biggest sets of changes into smaller commits. Smaller commits tend to be easier to review in general (so long as they're coherent); and the nature of these changes makes them especially sensitive to commit size, for reasons I explain in the last comment below.

Tha last comment is about one place to split: the biggest commit, which appears to correspond to a squash of the two biggest commits from the previous revision.

And then I think in fact it would help to split those two biggest commits up somewhat further. For example, in compose_box it could be validations; then the upload/attach flows; then other strings, including buttons and hint text.

gnprice · 2023-09-29T23:38:25Z

lib/api/exception.dart

-  /// property in the JSON response, and is translated into the user's language.
+  /// property in the JSON response.
  final String message;


Now that this is remaining a String, the old comment remains helpful.

gnprice · 2023-09-29T23:42:31Z

test/widgets/action_sheet_test.dart

+      localizationsDelegates: ZulipLocalizations.localizationsDelegates,
+      supportedLocales: ZulipLocalizations.supportedLocales,


api: Add translations for exceptions related to api requests Also hook up some other test suites that require localization contexts for tests to pass given this change.

These test changes are no longer needed in this commit, right?

(I think they might each be needed in a later commit.)

bump — I meant all three of the test changes mentioned in the quoted commit message :)

gnprice · 2023-09-29T23:44:31Z

test/api/core_test.dart

+    checkRequest((foo: 'bar'), it()
+      ..message
+      .equals(zulipLocalizations.errorNetworkRequestFailed));


nit:

Suggested change

checkRequest((foo: 'bar'), it()

..message

.equals(zulipLocalizations.errorNetworkRequestFailed));

checkRequest((foo: 'bar'), it()

..message.equals(zulipLocalizations.errorNetworkRequestFailed));

That's how we usually write this; it's a bit more compact even here, and it's especially helpful when there are several such cascades .. on the same Subject.

gnprice · 2023-09-29T23:46:55Z

lib/model/localizations.dart

@@ -0,0 +1,6 @@
+import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
+
+abstract final class GlobalLocalizations {


nit:

model: Add GlobalLocalization class for static access to localizations

commit message should match code on name of class

gnprice · 2023-09-29T23:54:01Z

assets/l10n/app_en.arb

+  "errorMessageDoesNotSeemToExist": "That message does not seem to exist.",
+  "@errorMessageDoesNotSeemToExist": {
+    "description": "Error message when loading a message that does not exist."
+  },
+  "errorQuotationFailed": "Quotation failed",
+  "@errorQuotationFailed": {
+    "description": "Error dialog message when quoting a message failed."


Let's pick a consistent term for these to use in descriptions — in particular in commit
491f21b action_sheet: Translations

we have "error message", "dialog message", and "error dialog message".

I think "error message" works well. Same reasoning as at #306 (comment) for not specifying "dialog".

gnprice · 2023-09-30T00:35:02Z

assets/l10n/app_en.arb

+  },
+  "loginAddAnAccount": "Add an account",
+  "@loginAddAnAccount": {
+    "description": "Page title for screen to add a zulip account."


Suggested change

"description": "Page title for screen to add a zulip account."

"description": "Page title for screen to add a Zulip account."

gnprice · 2023-09-30T00:43:05Z

assets/l10n/app_en.arb

+  "loginAddAnAccountConfirm": "Continue",
+  "@loginAddAnAccountConfirm": {
+    "description": "Button label to submit form for adding an account."


This name and description feel like they're going to puzzle some translators because they seem at variance with the word that makes up the English version.

In the UI where this is used, the reason "Continue" is appropriate is that this is really just one step of a form — it's the page where you enter what server you want to log into, and this button leads to the next page which gives you a choice of how to log in. (Well, for now, it leads straight to the email/password screen, but after #36 it'll offer more options.)

I think it'd make sense to make this more general — if we end up with several screens in different parts of the app that have similar "advance to next screen" flows, we'll want the button to get the same label on each one.

gnprice · 2023-09-30T00:45:00Z

assets/l10n/app_en.arb

+  "@loginEmailLabel": {
+    "description": "Label for input when an email is required to login."
+  },
+  "loginErrorMissingEmail": "Please enter your email",


This had a final period, so it should keep it (at least as part of a commit like this that's about restructuring things, not changing the visible UI).

Similarly for username and password.

gnprice · 2023-09-30T00:45:30Z

assets/l10n/app_en.arb

+  "@loginHidePassword": {
+    "description": "Icon label for button to hide password in input form."
+  },
+  "loginEmailLabel": "Email",


This was "Email address".

gnprice · 2023-09-30T00:50:46Z

lib/widgets/compose_box.dart

@@ -48,12 +48,12 @@ enum TopicValidationError {
  mandatoryButEmpty,
  tooLong;



The changes in this file happen in a commit that says it's all about login:

login: Translations --- assets/l10n/app_en.arb | 238 +++++++++++++++++++++++++++++++++++++++++++++-- lib/widgets/compose_box.dart | 115 +++++++++++++---------- lib/widgets/login.dart | 53 ++++++----- 3 files changed, 330 insertions(+), 76 deletions(-)

A previous revision had separate commits for this file and login.dart, so they must have gotten squashed.

Both files' changes are pretty big — bigger than any of the others in this series, in fact, it looks like — so the combined size does make it noticeably harder to read and review. It'd be good to split them back up.

One fact about the nature of these changes which makes their readability especially sensitive to the size of the commit is that each small piece of the change naturally has two halves, located in widely separated places, which need to match up. So in order to review any given small piece of the change, the reader has to go and look up its other half, and view both of them at the same time (or at a minimum flip back and forth between them). The bigger the commit, the more there is to search through to find that other half; so the total review work is roughly quadratic in the size of the commit.

(If we did keep these squashed, though, we'd definitely want to edit the commit message to reflect its actual contents.)

This was two commits but got squashed (I believe unintentionally). I've broken it up into further conceptual pieces.

sirpengi · 2023-10-02T18:24:33Z

@gnprice Thanks for the comments as always. This is ready for review again!

gnprice

Thanks @sirpengi for the revision! This is very close now to merge — just a few comments on these changes, all minor. I appreciate the additional splitting-up of the commits; that was quite helpful for reading them.

There are also some small wording tweaks I have for the descriptions, which I can just do as a commit on top at merge time.

The one other comment I have is that this PR shouldn't be marked as closing #277, because there are still some hard-coded English strings left — see in particular git grep TODO.i18n and git grep 'Text\(['\''"]' lib/. But let's leave those out of scope for this PR. Perhaps the right approach is that after we merge this, I'll take a turn going through the remaining strings; that will also be good for spreading around our experience with the workflow.

gnprice · 2023-10-02T18:55:29Z

test/widgets/action_sheet_test.dart

+      localizationsDelegates: ZulipLocalizations.localizationsDelegates,
+      supportedLocales: ZulipLocalizations.supportedLocales,


bump — I meant all three of the test changes mentioned in the quoted commit message :)

gnprice · 2023-10-02T20:36:18Z

assets/l10n/app_en.arb

+  "loginAddAnAccount": "Add an account",
+  "@loginAddAnAccount": {
+    "description": "Page title for screen to add a Zulip account."


I like the convention you've used elsewhere of having the name end in "page title" when it's the title of a page. Let's apply that consistently here, too.

bump -- "page title" :-)

gnprice · 2023-10-02T20:43:43Z

lib/widgets/action_sheet.dart

-        errorMessage = 'That message does not seem to exist.';
+        errorMessage = zulipLocalizations.errorMessageDoesNotSeemToExist;


nit: These three commit messages should have a verb:

8b9ea0f action_sheet: Translations
159c854 dialog: Translations
d4b0cd2 lightbox: Translations

"Add translations", like in several of your other commits, would do nicely.

sirpengi · 2023-10-04T13:04:38Z

@gnprice ready again for review!

… permission issues

Some grammatical fixes, some clarifications, some small corrections.

gnprice · 2023-10-09T21:34:52Z

Thanks for the revison! Merging, with that added commit on top tweaking some of the descriptions.

(And I guess the small last round of review that your last revision was in response to didn't make it into the GitHub thread's main timeline; but it's up at #306 (comment) .)

I'll also go add myself as an assignee on the issue #277, and will plan to pick up the remaining part of it soon.

sirpengi requested review from gnprice and chrisbobbe September 20, 2023 22:03

sirpengi force-pushed the pr-localization-sweep branch from 891c717 to baca2c9 Compare September 25, 2023 20:25

gnprice reviewed Sep 26, 2023

View reviewed changes

sirpengi force-pushed the pr-localization-sweep branch 4 times, most recently from 3cbfccf to 629e84a Compare September 26, 2023 15:47

gnprice reviewed Sep 27, 2023

View reviewed changes

sirpengi force-pushed the pr-localization-sweep branch 3 times, most recently from f042fe4 to 4a87936 Compare September 29, 2023 21:51

gnprice reviewed Sep 30, 2023

View reviewed changes

sirpengi force-pushed the pr-localization-sweep branch 2 times, most recently from 9fd9e65 to da9daa0 Compare October 2, 2023 18:23

gnprice reviewed Oct 2, 2023

View reviewed changes

sirpengi force-pushed the pr-localization-sweep branch from da9daa0 to c762e7b Compare October 4, 2023 13:01

sirpengi force-pushed the pr-localization-sweep branch from c762e7b to 8f04e4b Compare October 9, 2023 09:19

sirpengi added 6 commits October 9, 2023 14:20

msglist [nfc]: Update i18n todo with specific issue

872740b

clipboard test [nfc]: Remove unnecessary TODO

01d875b

l10n [nfc]: Condense placeholder definitions in app_en.arb

c3eba46

api: Add toString method to ApiRequestException

e497625

model: Add GlobalLocalizations class for static access to localizations

2977d4f

api: Add translations for exceptions related to api requests

09a2852

sirpengi and others added 11 commits October 9, 2023 14:20

action_sheet: Add translations

25409df

dialog: Add translations

f96e5ff

lightbox: Add translations

772524d

login: Add translations for ServerUrlValidationError enum

503d66d

compose: Add translations for TopicValidationError enum

82fb6e2

compose: Add translations for ContentValidationError enum

12bff8b

login: Add translations

699d8fd

compose: Add translations in _uploadFiles

c492707

compose: Add translations for attaching media to messages and related…

a6d57d2

… permission issues

compose: Add translations to UI widgets

84a30b9

i18n [nfc]: Tweak wording of descriptions for various strings

7f464ae

Some grammatical fixes, some clarifications, some small corrections.

gnprice force-pushed the pr-localization-sweep branch from 8f04e4b to 7f464ae Compare October 9, 2023 21:35

gnprice merged commit 7f464ae into zulip:main Oct 9, 2023
1 check passed

sirpengi deleted the pr-localization-sweep branch January 23, 2024 14:17

		// TODO(i18n)
		copyWithPopup(context: context, successContent: const Text('Text copied'),

		/// For [ZulipApiException] this may be supplied by the server as the `message`
		/// property in the JSON response.

		String get label;
		String Function(ZulipLocalizations) get translateLabel;

		/// For [ZulipApiException] this is supplied by the server as the `message`
		/// property in the JSON response, and is translated into the user's language.

		ApiRequestException({required this.routeName, required this.message});

		}

	"composeBoxSelfDMHint": "Jot down something",
	"composeBoxSelfDmHint": "Jot down something",

		localizationsDelegates: ZulipLocalizations.localizationsDelegates,
		supportedLocales: ZulipLocalizations.supportedLocales,

		@@ -0,0 +1,6 @@
		import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';

		abstract final class GlobalLocalizations {

	"description": "Page title for screen to add a zulip account."
	"description": "Page title for screen to add a Zulip account."

		@@ -48,12 +48,12 @@ enum TopicValidationError {
		mandatoryButEmpty,
		tooLong;

		errorMessage = 'That message does not seem to exist.';
		errorMessage = zulipLocalizations.errorMessageDoesNotSeemToExist;

Localization sweep #306

Localization sweep #306

Conversation

sirpengi commented Sep 20, 2023 • edited Loading

gnprice commented Sep 21, 2023

sirpengi commented Sep 25, 2023

gnprice 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

sirpengi commented Sep 26, 2023

gnprice left a comment • 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

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

sirpengi commented Sep 29, 2023

gnprice 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

sirpengi commented Oct 2, 2023

gnprice 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

sirpengi commented Oct 4, 2023

gnprice commented Oct 9, 2023

sirpengi commented Sep 20, 2023 •

edited

Loading

gnprice left a comment •

edited

Loading