feat: implement shellcheck lints in command blocks (#191) #264

thatRichman · 2024-12-08T20:05:56Z

This pull request adds a new rule to wdl-lint.

Rule Name: ShellCheckRule

Describe the rules you have implemented and link to any relevant issues.

I have implemented a lint rule that runs shellcheck against command sections and reports the comments as diagnostics. This is to address #191 .

Before submitting this PR, please make sure:

You have added a few sentences describing the PR here.
You have added yourself or the appropriate individual as the assignee.
You have added at least one relevant code reviewer to the PR.
Your code builds clean without any errors or warnings.
You have added an entry to the relevant CHANGELOG.md (see
["keep a changelog"] for more information).
Your commit messages follow the [conventional commit] style.

Rule specific checks:

You have added the rule as an entry within RULES.md.
You have added the rule to the rules() function in wdl-lint/src/lib.rs.
You have added a test case in wdl-lint/tests/lints that covers every
possible diagnostic emitted for the rule within the file where the rule
is implemented.
If you have implemented a new Visitor callback, you have also
overridden that callback method for the special Validator
(wdl-ast/src/validation.rs) and LintVisitor
(wdl-lint/src/visitor.rs) visitors. These are required to ensure the new
visitor callback will execute.
You have run wdl-gauntlet --refresh to ensure that there are no
unintended changes to the baseline configuration file (Gauntlet.toml).
You have run wdl-gauntlet --refresh --arena to ensure that all of the
rules added/removed are now reflected in the baseline configuration file
(Arena.toml).

thatRichman · 2024-12-08T20:06:48Z

There's still a lot of testing and polishing to do, which will undoubtedly reveal some issues, but I wanted to open for comments before I went any further in case I'm way off from what's desired.

thatRichman · 2024-12-08T20:16:28Z

wdl-lint/src/rules/command_shellcheck.rs

+const SHELLCHECK_BIN: &str = "shellcheck";
+
+/// shellcheck lints that we want to suppress
+const SHELLCHECK_SUPPRESS: &[&str] = &[


This is a copy-paste of miniwdl's list. I think we can remove most of these since most were added to work around their use of dummy literals, which we're not (currently) doing.

Yeah I think this is good for now. It would be nice to make these configurable in the future. Perhaps you could add a TODO comment above it and/or an issue when this is merged?

Not perfect, but users can add #shellcheck disable= directives right into the comment of a command section. Or use SHELLCHECK_OPTS to globally disable.

wdl-lint/src/rules/command_shellcheck.rs

a-frantz

Looks like this is on the right track! Didn't go through it with a fine-tooth comb yet, but left some comments/Qs on things that jumped out at me.

wdl-lint/src/rules/command_shellcheck.rs

claymcleod

Looking really good!

wdl-lint/Cargo.toml

wdl-lint/src/lib.rs

claymcleod · 2024-12-08T21:16:35Z

wdl-lint/src/rules/command_shellcheck.rs

+const SHELLCHECK_BIN: &str = "shellcheck";
+
+/// shellcheck lints that we want to suppress
+const SHELLCHECK_SUPPRESS: &[&str] = &[


Yeah I think this is good for now. It would be nice to make these configurable in the future. Perhaps you could add a TODO comment above it and/or an issue when this is merged?

wdl-lint/src/rules/command_shellcheck.rs

wdl-lint/src/util.rs

wdl-lint/tests/lints/command-shellcheck-ok/source.errors

wdl-lint/src/rules/command_shellcheck.rs

wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors

a-frantz

This is looking great so far. Really excited to get this feature merged in 😎

wdl-lint/src/rules/shellcheck.rs

a-frantz · 2024-12-10T14:06:02Z

wdl-lint/src/rules/shellcheck.rs

+        "ShellCheck (https://shellcheck.net) is a static analysis tool and linter for sh / bash. \
+        The lints provided by ShellCheck help prevent common errors and \
+        pitfalls in your scripts. Following its recommendations will increase \
+        the robustness of your command sections."


Just want to say good job writing this explanation() text. I think it's spot on 👍

wdl-lint/src/rules/shellcheck.rs

wdl-lint/tests/lints/shellcheck-error/source.errors

a-frantz · 2024-12-10T15:01:46Z

wdl-lint/src/rules/shellcheck.rs

+                state.exceptable_add(
+                    Diagnostic::error("running `shellcheck` on command section")
+                        .with_label(
+                            "could not find `shellcheck` executable.",
+                            command_keyword.text_range().to_span(),
+                        )
+                        .with_rule(ID)
+                        .with_fix("install shellcheck or disable this lint."),
+                    SyntaxElement::from(section.syntax().clone()),
+                    &self.exceptable_nodes(),
+                );


Making a comment to open the floor for discussion.

Is this how we want to handle the case that SC isn't installed?

If we want to create a diagnostic that ShellCheck couldn't be run, it should be a note I think. Not an error. Errors should be reserved for broken WDL, and this isn't that. I could maybe see this at warning level? But I'd prefer note personally.

Putting myself in the shoes of a naive user, if I've never heard of ShellCheck I'm just disabling this lint going forward. After all, we are asking folks to install another dependency without offering them any guidance on how to do that. And it would potentially annoy me seeing this every time I forget to disable the shellcheck rule. I think we need some more here. A link to shellcheck.net? And a one sentence sales pitch on the benefit of the SC?

I thought we decided that ShellCheck was only going to be an opt-in rule? I think the behavior depends largely on that decision. I like making it opt-in because it delineates the behavior between silent failure because of the missing tool and just being turned off.

If the rule is enabled by default, I agree that it should be silent so that every user doesn't have to disable it. I don't think this is what we should do though.

If the rule is disabled by default and is opt-in only, then I think it should alert you with a warning that you are missing the required binary for this rule (it's probably not a note because, if you explicitly enabled this rule, you're probably expecting it to be installed).

I thought we decided that ShellCheck was only going to be an opt-in rule? I think the behavior depends largely on that decision. I like making it opt-in because it delineates the behavior between silent failure because of the missing tool and just being turned off.

Maybe I missed something then. But the complication with making it opt-in is that this would be the first rule like that, and would probably require some tinkering of the architecture. We don't have a config system yet, so how exactly do we "opt-in"? Is that OOS for this PR?

Well, I was thinking that this wouldn't be in the default set, and we would just add something in Sprocket that injects this rule in on a flag or similar. But, that being said, I'm fine with keeping it in the default set with no diagnostic on missing, so long as the TODO is marked in an issue (and, I would ask that, if we're close to having that working, we include it in the commit just commented out for now so it's easy to enable when we're ready).

I'll go ahead and leave it in the default set but remove the diagnostic message on missing. I'll just comment out the diagnostic as requested (will fix up the wording a bit based on other feedback).

wdl-lint/src/rules/shellcheck.rs

thatRichman · 2024-12-12T03:00:00Z

Rebased. Gauntlet in arena mode is giving me the following:

thread 'main' panicked at /home/spencer/projects/wdl/gauntlet/src/lib.rs:258:25:
duplicate diagnostic: `ww-fastq-to-cram.wdl:101:20: note[ShellCheck]: `shellcheck` reported the following diagnostic` at /ww-fastq-to-cram.wdl:101

so I'll look into that tomorrow.

a-frantz · 2024-12-12T14:15:23Z

@thatRichman do you think you can figure out how to install shellcheck on the Arena CI runner? I'd rather not merge this until we can see the lints that will show up "in the wild". I won't be surprised if we find another SC rule or two should be disabled as being unhelpful in the context of a WDL command block.

Feel free to ping me on Zulip if that poses any issues! I imagine the windows runner could pose a challenge 🤔

thatRichman · 2024-12-12T14:27:54Z

@thatRichman do you think you can figure out how to install shellcheck on the Arena CI runner? I'd rather not merge this until we can see the lints that will show up "in the wild". I won't be surprised if we find another SC rule or two should be disabled as being unhelpful in the context of a WDL command block.

Feel free to ping me on Zulip if that poses any issues! I imagine the windows runner could pose a challenge 🤔

@a-frantz Sure, shouldn't be too bad. Shellcheck provides a pre-built windows binary that we can pull from their releases. Might just take a little fiddling.

thatRichman · 2024-12-13T21:37:22Z

@a-frantz Here's a link to an example of a new install-shellcheck action running on my fork. One thing to note is Ubuntu being Ubuntu only has 0.8.0 by default. macOS and Windows will always run latest. Maybe a good thing to be testing across versions. But if we want to forego the apt caching we could just pull from GH the same as Windows.

edit: it's also suspicious that Arena passed on Windows when the duplicate diagnostic bug was still present. Makes me think it probably isn't finding the executable.

thatRichman · 2024-12-13T21:48:08Z

wdl-lint/src/util.rs

@@ -24,6 +34,22 @@ pub fn is_inline_comment(token: &Comment) -> bool {
    false
 }

+/// Determines if a string containing embedded quotes is properly quoted.


Suggested change

/// Determines if a string containing embedded quotes is properly quoted.

/// Determines whether or not a string containing embedded quotes is properly quoted.

claymcleod · 2024-12-14T23:19:14Z

@a-frantz Here's a link to an example of a new install-shellcheck action running on my fork. One thing to note is Ubuntu being Ubuntu only has 0.8.0 by default. macOS and Windows will always run latest. Maybe a good thing to be testing across versions. But if we want to forego the apt caching we could just pull from GH the same as Windows.

edit: it's also suspicious that Arena passed on Windows when the duplicate diagnostic bug was still present. Makes me think it probably isn't finding the executable.

My two cents: let's just always pull from latest on everything (including Mac). It makes it a bit more straightforward to maintain the Github action associated with that, and it also ensures that every platform always works with the bleeding edge Shellcheck.

thatRichman · 2024-12-14T23:38:09Z

I'm starting to see why the miniwdl folks went with literals instead of variables / expansions when substituting placeholders.

The common false positives I'm seeing from Arena are:

SC2076

❌ read_group.wdl:200:43: note[ShellCheck]: Remove quotes from right-hand side of =~ to match as a regex rather than literally.

caused by:

| [[ ~{read_group.ID} =~ ~{restrictive_pattern} ]]

because we're replacing both of these placeholders with double-quoted expansions.

and SC2001

❌ util.wdl:132:9: note[ShellCheck]: See if you can use ${variable//search/replace} instead.

caused by:

echo ~{string} | sed 's/~{delimiter}/\n/g' > split_strings.txt

because with a bash variable you can use the suggested syntax but that obviously doesn't apply to WDL placeholders.

as well as SC2066:

❌ samtools.wdl:577:21: note[ShellCheck]: Since you double quoted this, it will not word split, and the loop will only run once.

caused by:

 for file in ~{sep(" ", bams)}

Though, this one is also basically identical to one that miniwdl has to suppress.

I've found (what I think) is an elegant solution to determine whether to use a literal or expansion, which reduces but does not eliminate false positives.

That being said, I think miniwdl's approach of guessing the placeholder's type and using an appropriate literal would be overly complicated here. So I think it comes down to the severity and frequency of the lints we end up needing to suppress. I still think (with the commit I'm about to push) that we're better off than always using literals. We could probably spend weeks working to get the false positives to an absolute minimum without suppressing anything, but that feels a bit bikesheddy to me. Of course I will defer.

a-frantz · 2024-12-15T13:35:47Z

I'm starting to see why the miniwdl folks went with literals instead of variables / expansions when substituting placeholders.

So am I. However with wdl-lint currently being based off of wdl-ast, we don't have a good way of knowing the right type of literal to inject. We've discussed potentially refactoring wdl-lint to be based off of wdl-analysis so that we'd have access to resolved type information. That refactor would certainly be OOS for this PR, but would potentially simplify this problem by a great amount.

I'm thinking we shouldn't expend a large amount of effort eliminating these false positives with the current AST-based implementation.

@thatRichman can you remove this rule from the default set of rules (as Clay originally suggested) so that this is opt-in only?

We should re-architect wdl-lint so that it can have the results of a full static-analysis and revisit this rule at that time. In the meantime, this rule can be in a bit of a beta stage. It's certainly still useful and worth merging into main, but the false-positive lints are too big an issue to include this in the default rule set IMO.

thatRichman · 2024-12-15T16:47:20Z

Sounds good. I'll disable the shellcheck-install action (once I get it working on all OSes) so that it be re-enabled at a later date.

I know the intent is to support non-default rules in sprocket eventually, but should I put any work into supporting non-default rules in the dev CLI in this PR? Seems OOS, so happy to throw in an issue if there isn't one already.

Also, I think I will wait for the resolution of #268 before making any final changes to this PR, as it throws off diagnostic placements.

a-frantz · 2024-12-17T16:17:20Z

@thatRichman

Also, I think I will wait for the resolution of #268 before making any final changes to this PR, as it throws off diagnostic placements.

Fixed! Thanks for logging that issue.

thatRichman · 2024-12-18T01:59:21Z

@a-frantz This should be ready for final review. One question I have is whether or not to run gauntlet with shellcheck enabled (I added this option), since this rule is in 'beta' and thus gauntlet will report missing lints if not run with --shellcheck (if it has previously been blessed while using --shellcheck).

Also, I fully intend to squash these 39 commits into 1. I can do that before or after final review, whatever you prefer.

a-frantz · 2024-12-18T02:12:18Z

Sweetness! Will give this a review in the morning. I also added the rest of the team as reviewers in case I miss something.

One question I have is whether or not to run gauntlet with shellcheck enabled (I added this option), since this rule is in 'beta' and thus gauntlet will report missing lints if not run with --shellcheck (if it has previously been blessed while using --shellcheck).

IMO we leave it out of gauntlet/arena for now. I imagine it is going to add a boatload of diagnostics, and we like to review each one. Considering we know upfront some portion will be false positives, I don't think it's worth the effort.

Also, I fully intend to squash these 39 commits into 1. I can do that before or after final review, whatever you prefer.

We squash as part of the merge, so you shouldn't need to do anything to deal with it.

gauntlet/src/lib.rs

wdl-lint/src/rules/shellcheck.rs

wdl/src/bin/wdl.rs

a-frantz · 2024-12-18T14:16:54Z

wdl-lint/src/rules/shellcheck.rs

+    let label = format!(
+        "SC{}[{}]: {}",
+        diagnostic.code, diagnostic.level, diagnostic.message
+    );
+    Diagnostic::note(&diagnostic.message)
+        .with_rule(ID)
+        .with_label(label, span)


This repeats the diagnostic.message twice. Is that desirable? Should we omit it from the label as being redundant, or do we like it appearing twice?

I'm conflicted. Pulling an example from below, I think the initial instance by the note is fine. The second provides the additional info link, which could be helpful. I do notice that the span isn't highlighted correctly. I also think the fix needs to be something else.

note[ShellCheck]: You need a space before the ]. ┌─ tests/lints/shellcheck-error/source.wdl:18:25 │ 18 │ if [ -f "$broken"] │ ^ │ │ SC1020[error]: You need a space before the ]. │ more info: https://www.shellcheck.net/wiki/SC1020 │ = fix: address the diagnostic as recommended in the message

@adthrasher Could you clarify what you mean about the span not being highlighted correctly? Shellcheck outputs this diagnostic as a single position one character ahead of the closing bracket.

#!/usr/bin/env bash # the below line has 11 columns of text [ -f "foo"]

yields:

{ "file": "test.sh", "line": 3, "endLine": 3, "column": 12, "endColumn": 12, "level": "error", "code": 1072, "message": "Missing space before ]. Fix any mentioned problems and try again.", "fix": null }

(note that shellcheck 1-indexes columns)
so I think it's 1:1 with how shellcheck reports it.

Re: the message

@peterhuene and I discussed this some in the Zulip. This initially came about because Gauntlet requires all diagnostic messages to be unique for a given line + position. The solution I came up with was to include the SC###[] in the message, but only the shellcheck message in the label. But happy to rework. Agree the fix could be much more useful.

So it's the underlying shellcheck that returns the column position after ]. That's good to know. It still seems like the highlight should be one position earlier, so that it flags the ].

a-frantz · 2024-12-18T14:20:31Z

@thatRichman can you add a test that #shellcheck disable= directives work as expected?

adthrasher · 2024-12-18T15:25:49Z

wdl-lint/src/rules/shellcheck.rs

+            format!("more info: {}/SC{}", &SHELLCHECK_WIKI, diagnostic.code),
+            span,
+        )
+        .with_fix("address the diagnostic as recommended in the message")


@a-frantz - I know you just added/updated our fix messages. What do you think of this one? The generic message isn't helpful to users, but the alternative would presumably involve parsing the warnings from shellcheck and writing a fix for each type.

To be perfectly honest, yall didn't really convince me that it's the right move for every diagnostic to include a fix. I was just outvoted and didn't feel strongly enough to put up a fight 😅

So my frank opinion is that this fix should be dropped along with some of our other "unhelpful" fix messages. But we've set the precedent that every diagnostic should include some kind of fix, and I'm not sure how we can improve this one without some rather intense work 🤷

@thatRichman I see here that shellcheck provides it's own fix message. Maybe we should just copy that? I haven't seen what they look like, so maybe they aren't of good quality 🤔
Can you share some examples of the shellcheck fix messages?

The fix messages describe the structural changes required to address the lint, and are not always available.
Example:

echo [ -z $foo ]

"fix": { "replacements": [ { "line": 4, "endLine": 4, "precedence": 7, "insertionPoint": "afterEnd", "column": 11, "replacement": "\"", "endColumn": 11 }, { "line": 4, "endLine": 4, "precedence": 7, "insertionPoint": "beforeStart", "column": 15, "replacement": "\"", "endColumn": 15 } ]

We could convert these into a human-readable message, but I can't say if we want to.

@a-frantz - This could be used to get toward my goal for fix. I would want to use that fix to print the correct line.

echo [ -z "$foo" ]

Oo. That's kind of messy... At the very least it wouldn't be trivial to convert that to something I would want us to print.

@adthrasher I agree that we should work towards having every fix message print the "corrected" code where possible. But that's not very easy to implement robustly. This one in particular looks tough to parse when it's present.

My two cents on this issue is we leave the fix message as Spencer has written it, but we can make a new issue to revisit this specific fix (and many of our other fixes for that matter).

@a-frantz - That particular example doesn't look too bad to convert to text. You've just got the line, column, replacement text, and how to place it. The bigger issue, to me, is that it isn't always present (though some errors probably aren't straightforward like this one). But I agree, we can leave it as-is and have a new issue to revisit.

wdl-lint/tests/lints/shellcheck-error/source.errors

thatRichman · 2024-12-18T22:14:55Z

wdl-lint/src/lib.rs

@@ -150,3 +152,32 @@ pub fn rules() -> Vec<Box<dyn Rule>> {

    rules
 }
+
+/// Gets the optional rule set.
+pub fn optional_rules() -> Vec<Box<dyn Rule>> {


this is probably overkill since this will (hopefully) be a temporary home for shellcheck.

gauntlet/src/lib.rs

a-frantz · 2024-12-19T19:57:11Z

We're pretty dang close here @thatRichman . Please get the CI all green, then I think this will have my approval
https://github.com/stjude-rust-labs/wdl/blob/main/CONTRIBUTING.md#the-ci-has-turned-red-how-do-i-make-it-green-again

The general rule we follow is to require approval from two core team members for a merge.

thatRichman · 2024-12-19T21:58:50Z

@a-frantz Sounds good. One thing to note is that I did end up needing to re-enable shellcheck install in the CI. This is because if shellcheck isn't present, the first lint test that is run will end up with the 'shellcheck not found' warning, and the source.errors for the shellcheck-* lints will be empty. I thought about modifying the lints test script to selectively skip the shellcheck lints, but this would not address the 'missing shellcheck' lint.

Here is an example of the modified CI going green on my repo but I don't think there's any way to reflect that here unless we merge the actions changes to main in a separate PR first.

thatRichman commented Dec 8, 2024

View reviewed changes

wdl-lint/src/rules/command_shellcheck.rs Outdated Show resolved Hide resolved

a-frantz reviewed Dec 8, 2024

View reviewed changes

claymcleod requested changes Dec 8, 2024

View reviewed changes

thatRichman commented Dec 9, 2024

View reviewed changes

wdl-lint/src/rules/command_shellcheck.rs Outdated Show resolved Hide resolved

thatRichman commented Dec 10, 2024

View reviewed changes

wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors Outdated Show resolved Hide resolved

thatRichman force-pushed the feat/shellcheck-lint-command-blocks branch from 958e222 to 8474ebd Compare December 10, 2024 13:56

a-frantz reviewed Dec 10, 2024

View reviewed changes

claymcleod self-requested a review December 11, 2024 02:02

thatRichman commented Dec 11, 2024

View reviewed changes

wdl-lint/src/rules/shellcheck.rs Show resolved Hide resolved

thatRichman force-pushed the feat/shellcheck-lint-command-blocks branch from 759c7f4 to 6f792a2 Compare December 11, 2024 23:46

thatRichman commented Dec 11, 2024

View reviewed changes

wdl-lint/src/rules/shellcheck.rs Outdated Show resolved Hide resolved

thatRichman force-pushed the feat/shellcheck-lint-command-blocks branch 2 times, most recently from b76e234 to f2506be Compare December 12, 2024 02:42

thatRichman marked this pull request as ready for review December 12, 2024 03:00

claymcleod assigned thatRichman Dec 12, 2024

thatRichman mentioned this pull request Dec 13, 2024

Possible CommandSection strip_whitespace regression, now removes the first word on the first line #268

Closed

thatRichman commented Dec 13, 2024

View reviewed changes

a-frantz mentioned this pull request Dec 17, 2024

refactor wdl-lint on top of wdl-analysis #273

Open

thatRichman added 5 commits December 17, 2024 20:58

chore: fmt

7edf514

chore: grammar

fead4cd

fix: add shellcheck lint rule to tests

47e911c

feat: add shellcheck option to gauntlet

e83d79a

chore: fmt

76ed909

thatRichman force-pushed the feat/shellcheck-lint-command-blocks branch from a7781eb to 76ed909 Compare December 18, 2024 01:58

fix: undo changes to CI.yml

91046ff

a-frantz requested review from peterhuene and adthrasher December 18, 2024 02:07

a-frantz linked an issue Dec 18, 2024 that may be closed by this pull request

feature request: incorporate shellcheck for linting command blocks #191

Open

fix: always use latest github release of shellcheck

d1bb4ae

chore: lints

58b8d67

a-frantz reviewed Dec 18, 2024

View reviewed changes

adthrasher reviewed Dec 18, 2024

View reviewed changes

wdl-lint/tests/lints/shellcheck-error/source.errors Show resolved Hide resolved

thatRichman added 4 commits December 18, 2024 16:33

chore: update wdl and gauntlet changelogs

5e942c5

fix: re-enable missing shellcheck diagnostic

d8dfef1

fix: include optional rules when checking rule validity

984dd70

chore: fmt

9cd70c0

thatRichman commented Dec 18, 2024

View reviewed changes

a-frantz reviewed Dec 19, 2024

View reviewed changes

gauntlet/src/lib.rs Outdated Show resolved Hide resolved

thatRichman added 4 commits December 19, 2024 15:23

chore: lint

7db4246

fix: only use shellcheck rule in arena mode

7b50b5b

chore: lint

3dfa8fd

fix: re-enable shellcheck install for CI

3a8e4ae

	/// Determines if a string containing embedded quotes is properly quoted.
	/// Determines whether or not a string containing embedded quotes is properly quoted.

feat: implement shellcheck lints in command blocks (#191) #264

Are you sure you want to change the base?

feat: implement shellcheck lints in command blocks (#191) #264

Conversation

thatRichman commented Dec 8, 2024 • edited by claymcleod Loading

thatRichman commented Dec 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a-frantz left a comment

Choose a reason for hiding this comment

claymcleod left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a-frantz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

claymcleod Dec 11, 2024 • 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

thatRichman commented Dec 12, 2024

a-frantz commented Dec 12, 2024

thatRichman commented Dec 12, 2024

thatRichman commented Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

claymcleod commented Dec 14, 2024

thatRichman commented Dec 14, 2024

a-frantz commented Dec 15, 2024

thatRichman commented Dec 15, 2024

a-frantz commented Dec 17, 2024

thatRichman commented Dec 18, 2024

a-frantz commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thatRichman Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

thatRichman Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a-frantz commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thatRichman Dec 18, 2024 • 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

a-frantz commented Dec 19, 2024

thatRichman commented Dec 19, 2024

thatRichman commented Dec 8, 2024 •

edited by claymcleod

Loading

claymcleod Dec 11, 2024 •

edited

Loading

thatRichman commented Dec 13, 2024 •

edited

Loading

thatRichman Dec 18, 2024 •

edited

Loading

thatRichman Dec 18, 2024 •

edited

Loading

thatRichman Dec 18, 2024 •

edited

Loading