STY: Apply pre-commit hooks to codebase #22809

matthewfeickert · 2022-04-08T22:03:27Z

PR Summary

This is a follow up PR to PR #21583 based on discussions in PR #22777 (c.f. #22777 (comment)) and on Gitter. This touches a lot of files so I'll walk through the story here, which should help motivate turning on pre-commit.ci.

The pre-commit hooks were added in PR Add pre-commit config and dev instructions #21583 (November 2021) with a pre-commit.ci

matplotlib/.pre-commit-config.yaml

Line 3 in a436253

autoupdate_schedule: 'quarterly'

though as pre-commit.ci wasn't turned on, there were no pre-commit hook updates coming in on that schedule and so I've run pre-commit autoupdate to update the pre-commit-hooks and flake8 ids.

With those updated, I then ran the failing (c.f. DEV: Add codespell to pre-commit hooks #22777 (comment)) pre-commit hooks and applied their necessary revisions as individual commits. So those are the style commits for end-of-file-fixer, trailing-whitespace, and check-docstring-first. These were also done as individual commits so that if in review any of these are viewed as not acceptable they can be dropped easily with a rebase.
As the following commits touch many files, this is a noisy first time run of the hooks. To help limit this, add a .git-blame-ignore-revs file that will automatically ignore commits in the blame view on GitHub (this file is required to be in the top level of the repo). This can also be done locally with git blame by running

git blame --ignore-revs-file .git-blame-ignore-revs

Example on my fork:

Note to reviewer: As this file contains commits it is sensitive to rebases, and so please make sure that these commits are correct if anyone rebases this during review.

This PR should probably be merged in advance of PR #22777 which can be trivially rebased and would be a good time to turn pre-commit.ci on.

pre-commit now passes the full repo:

$ pre-commit run --all-files
check for added large files..............................................Passed
check docstring is first.................................................Passed
fix end of files.........................................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
flake8...................................................................Passed

PR Checklist

Tests and Styling

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

[n/a] New features are documented, with examples if plot related.
[n/a] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
[n/a] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs should build without error).

matthewfeickert · 2022-04-08T22:29:23Z

.git-blame-ignore-revs

+# style: end-of-file-fixer pre-commit hook
+b372d66ab9e9aa30573f240d87a6ea92474cce75
+
+# style: trailing-whitespace pre-commit hook
+0f0a977d8aaba773570dff8f828c358df22203aa
+
+# style: check-docstring-first pre-commit hook
+1a36dc83af39159749ce5857eca6ddacd345e6bb


Note to reviewer: As this file contains commits it is sensitive to rebases, and so please make sure that these commits are correct if anyone rebases this during review.

This is great that this is possible. Really helpful for big cosmetic changes like this.

I rebased, but I also updated the commits.

$ git log --graph --oneline --decorate --all | head -n 10 * 02fc0611cf (HEAD -> mnt/apply-pre-commit-to-all-files, origin/mnt/apply-pre-commit-to-all-files) DOC: Add pre-commit hook run dev instructions * f3aa826d83 DEV: Add .git-blame-ignore-revs to ignore style in blame view * 66c12f2778 STY: Apply check-docstring-first pre-commit hook * 33516082c5 STY: Apply trailing-whitespace pre-commit hook * 94d985a744 STY: Apply end-of-file-fixer pre-commit hook * 7e04925fcd MNT: Update pre-commit hooks to latest versions | * 3dd6049160 (mnt/add-codespell-to-pre-commit-hooks) FIX: Fix typo caught by codespell pre-commit hook | * 19987645ec DEV: Add codespell to pre-commit hooks |/ * 19992285cb (upstream/main, origin/main, main) Merge pull request #22807 from anntzer/qd

matthewfeickert · 2022-04-08T22:47:30Z

doc/devel/development_setup.rst

+The hooks can also be run manually. All the hooks can be run, in order as
+listed in ``.pre-commit-config.yaml``, against the full codebase with ::
+
+    pre-commit run --all-files
+
+To run a particular hook manually, run ``pre-commit run`` with the hook id ::
+
+    pre-commit run <hook id> --all-files


You can of course target particular files with --file (e.g. pre-commit run --file README.rst) but given that is already done on the files that you're attempting to commit when you have pre-commit installed locally this didn't really seem important to note.

From pre-commit run --help:

--files [FILES ...] Specific filenames to run hooks on.

matthewfeickert · 2022-04-08T23:04:58Z

To address a question from Gitter:

In PR #17143 reviewdog was added. reviewdog currently appears to run linting for only flake8 (please correct me if I'm wrong). As to how pre-commit differs, it applies all the pre-commit hooks (including flake8) and will quarterly (as defined in the config)

matplotlib/.pre-commit-config.yaml

Line 3 in a436253

autoupdate_schedule: 'quarterly'

open a PR to automatically update the hooks that have new releases. It also runs all of the pre-commit hooks faster then reviewdog (though this really doesn't matter at all when you're comparing against how long the rest of the CI takes).

I'll leave it up to the dev team if this is sufficiently interesting/useful to have.

matthewfeickert · 2022-04-08T23:37:14Z

I also enabled pre-commit.ci on my fork to do a demo of what this looks like. c.f. matthewfeickert#9 which shows things working, and then also a little autofix demo from a comment.

oscargus · 2022-04-09T08:11:32Z

I'm not sure that we should require pre-commit hooks (as I take is an indirect suggestion here). While I tend to use it, there are setups where it is not really convenient. For example, I have different shells on Windows, one for running git (https://gitforwindows.org/) and one for Python (conda). While I can of course unify them (being too lazy so far, and doing much development on Linux), this is a bit of extra work and nothing I think should be an obstacle for contributing.

Also, I am not sure how this works when doing commits from graphical interfaces (which for some reason seems to be a popular way of using git...).

Running the hooks on all files is as you say a one-time effort. IMHO I do not think we need to use the .git-blame-ignore-revs here, but just go for it (if we should do it). All newly committed code will have these checks anyway and if someone touches some code and have white-space removal in the editor, it will be enforced anyway. With that said, I am not against cleaning the code in one go, but think the .git-blame-ignore-revs is really not required.

To me, pre-commit hooks primarily serve the purpose of not having to run the code quality tests locally and get warnings if something is not right, so I am not a big fan of the hooks actually changing stuff. Better to "learn" how it should be done.

matthewfeickert · 2022-04-09T08:56:52Z

I'm not sure that we should require pre-commit hooks (as I take is an indirect suggestion here).

This doesn't require developers to use them. Using pre-commit.ci is just going to find things that would later require a developer to go fix to and alert the PR contributor and the reviewers to them.

While I tend to use it, there are setups where it is not really convenient. For example, I have different shells on Windows, one for running git (https://gitforwindows.org/) and one for Python (conda). While I can of course unify them (being too lazy so far, and doing much development on Linux), this is a bit of extra work and nothing I think should be an obstacle for contributing.

I don't think I understand. :? What is the obstacle? Are you saying that pre-commit doesn't work on your Windows setup? Can you elaborate?

Also, I am not sure how this works when doing commits from graphical interfaces (which for some reason seems to be a popular way of using git...).

That is an interesting point that I've never thought about before. I would be a bit surprised (maybe naively?) to find out that popular Git GUIs have no plugin support though, and so I would (definitely naively) hope things "just work".

Running the hooks on all files is as you say a one-time effort. IMHO I do not think we need to use the .git-blame-ignore-revs here, but just go for it (if we should do it). All newly committed code will have these checks anyway and if someone touches some code and have white-space removal in the editor, it will be enforced anyway. With that said, I am not against cleaning the code in one go, but think the .git-blame-ignore-revs is really not required.

It is certainly not required, but the diff is pretty ugly and meaningless, so this was more of a "making it less painful" effort. So I'm quite happy to remove this, but the blame avoidance seems almost custom designed for these huge diffs situations.

To me, pre-commit hooks primarily serve the purpose of not having to run the code quality tests locally and get warnings if something is not right, so I am not a big fan of the hooks actually changing stuff. Better to "learn" how it should be done.

I take a different view on their purpose (automate the boring stuff that would need to get human changed anyway), but my opinion on that isn't the relevant thing here. The hooks only run locally if the developer has them installed locally (which is totally optional) and if a developer pushes code that fails pre-commit.ci then as automatic pushing back fixes is disabled

matplotlib/.pre-commit-config.yaml

Line 2 in a436253

autofix_prs: false

they can speed up the process and avoid having a reviewer have to ask for them for changes by just commenting "pre-commit.ci autofix". Nothing is happening automatically unless a developer opts in by using pre-commit locally or by asking pre-commit.ci to fix things for them. (I think I am probably telling you things that you already know though, so apologies here. 😬 Can you elaborate/clarify your points if I've misunderstood?)

I'll note also that this PR is simply enforcing the pre-commit hooks that were added in PR #21583, and if the dev team isn't happy with that hook selection I'm certainly happy to fix that here to whatever is desired. 👍

jklymak

With the git-blame-ignore-revs, I think these cosmetic changes are fine to get the recommit hooks to pass.

oscargus · 2022-04-09T10:18:23Z

I don't think I understand. :? What is the obstacle? Are you saying that pre-commit doesn't work on your Windows setup? Can you elaborate?

The problem is that the git (for Windows) bash shell and the (mini)conda shell (Anaconda prompt) don't have the same Python environment, so if I run git in the conda shell I have everything I need for the pre-commit, but not the nice aspects of the git bash shell (like branch name and tab-completion), and if I use the git shell, I do not have the conda environment.

As I said, this can be sorted out with a bit of effort and is just a convenience thing really, but I would assume it is a similar situation for the graphical clients as conda typically doesn't set up any environment variables except for it's own shell.

(I think I have three different Python installs on my Windows machine: conda, that I use for most things, Windows Python that ends up in the path, and the one provided with the MS dev-prompt for building certain stuff. So while I can have all three synced, it is not very practical...)

greglucas

I'm actually surprised there is this little churn in the files, it is almost all trailing whitespace.

I also prefer having the .git-blame-ignore-revs for the whitespace changes because what I'm really interested in with the blame is looking for who updated the actual code piece and I don't want to ping @matthewfeickert to fix something he only did a whitespace update on :)

greglucas · 2022-04-09T13:23:14Z

lib/matplotlib/backend_tools.py

@@ -989,12 +989,12 @@ def trigger(self, *args, **kwargs):
                 'help': ToolHelpBase,
                 'copy': ToolCopyToClipboardBase,
                 }
-"""Default tools"""
+# Default tools


Just delete this comment and the one below, the comment is the same as the variable name :)

Done in rebase I just pushed. 👍

matthewfeickert · 2022-04-09T16:11:17Z

Just showing that things still pass after rebase:

$ pre-commit run --all-files
check for added large files..............................................Passed
check docstring is first.................................................Passed
fix end of files.........................................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
flake8...................................................................Passed

oscargus · 2022-04-10T08:17:25Z

lib/matplotlib/backend_tools.py

@@ -989,12 +989,10 @@ def trigger(self, *args, **kwargs):
                 'help': ToolHelpBase,
                 'copy': ToolCopyToClipboardBase,
                 }
-"""Default tools"""


Why are these removed? Not sure if they are actually rendered, but those are doc-strings for the attributes.

They were removed given @greglucas review comment here: #22809 (comment)

They don't show up in the "files changed" tab during review and only show up in the "Conversation" tab because I rebased the PR.

OK! Yes, considering that the doc-string is the same as the name and that they do not show up in the rendered documentation it may make sense. (Not having them as a comment for sure makes sense.)

I was a bit worried that this indicates that one cannot really have variable doc-strings though. That those will be interpreted as "multiple top-level doc-strings".

Sorry, I was wrong. This change leads to a modification of the documentation. If that is wanted or not, I cannot really tell, but I do not think it is good if it was enforced by some pre-commit setting.

Current:

https://matplotlib.org/devdocs/api/backend_tools_api.html?highlight=default_tools#matplotlib.backend_tools.default_tools

After this PR it is removed:

https://output.circle-artifacts.com/output/job/fe50f888-9db6-41e2-836d-a7446e4b516f/artifacts/0/doc/build/html/search.html?q=default_tools

https://output.circle-artifacts.com/output/job/fe50f888-9db6-41e2-836d-a7446e4b516f/artifacts/0/doc/build/html/api/backend_tools_api.html?highlight=backend_tools#module-matplotlib.backend_tools

@oscargus I'll let you and the other members of the dev team make a call here, but the fact that this does show up in the current docs looks unintentional to me given the render. At least I don't see their value here in the docs, but I'm also obviously not an expert in this section of the API.

These docstring comments were added in 2015 back in PR #3652 by @fariza. So while I sure hope no one ever asks me to explain code that I wrote over 5 years ago 😬 maybe they can take a look and give their thoughts.

@greglucas as you suggested removal maybe you can also give your thoughts here.

FYI, check-docstring-first had a long-standing issue for these false positives, see pre-commit/pre-commit-hooks#159. It can be turned off per-file, and does catch the common case where imports sneak above the docstring. Or it can be dropped, either way.

I think this affects excess docstrings in examples far more than variable docstrings in the actual library, so I think I'm net positive on keeping it.

so I think I'm net positive on keeping it.

(Sorry, long week and my brain is a bit fuzzy.) Positive on keeping the check-docstring-first hook, correct?

I'm pretty indifferent on this (as it sounds like most of us are!). I'd lean towards sticking with what you have currently (removing the docstring/comment [and because of that removing the sphinx generated documentation] and leaving the pre-commit check in place).

@QuLogic @oscargus @jklymak @greglucas @tacaswell As it seems that there are no strong feelings against keeping the check-docstring-first hook and some positive leanings towards it, does anyone have any further questions on this PR?

To keep the noise down on this I've stopped rebasing it against main but I have a branch on my fork where I've continued to do so and pre-commit run --all-files is still passing with this rebased on main's HEAD.

oscargus · 2022-04-10T08:18:42Z

I don't want to ping

That is the general reason, yes. However, looking at the changes here: do you really expect that you will have to ping for any of these changes? They are primarily in non-code and the very few that are in the code are quite non-critical.

greglucas · 2022-04-10T14:31:15Z

I agree that this specific change isn't too invasive and wouldn't be a big issue either way for the ignore commits from my perspective. However, I see this as setting a good precedent for potential future updates dropping big style changes and makes it easy to see how to do it properly by just adding the commit hash to this file.

matthewfeickert · 2022-04-11T00:47:42Z

However, I see this as setting a good precedent for potential future updates dropping big style changes and makes it easy to see how to do it properly by just adding the commit hash to this file.

Yeah. 👍 I got the idea from @henryiii using it in scikit-build/scikit-build#685 where they use Black. I know that Matplotlib doesn't use any formatters like that, but if there was ever a big style revision it could be done with a diminished disruption with respect to using the Git history.

tacaswell

I have a slight preference that the LICENCES files not be touched / be ignored by these tools but not a big enough concern to block over it.

matthewfeickert · 2022-04-13T23:00:51Z

I have a slight preference that the LICENCES files not be touched / be ignored by these tools but not a big enough concern to block over it.

@tacaswell That can easily happen. They can just get added to the exclude list

matplotlib/.pre-commit-config.yaml

Lines 4 to 12 in c33557d

    
           exclude: | 
        
             (?x)^( 
        
               extern| 
        
               lib/matplotlib/mpl-data| 
        
               doc/devel/gitwash| 
        
               doc/users/prev| 
        
               doc/api/prev| 
        
               lib/matplotlib/tests/tinypages 
        
               )

I'll do that, rebase to get the right commits, and then request re-review.

Requested by Thomas Caswell in PR 22809

* Update pre-commit hooks: - https://github.com/pre-commit/pre-commit-hooks: v4.0.1 → v4.2.0 - https://github.com/pycqa/flake8: v3.9.2 → v4.0.1 This is being applied manually and not waiting for pre-commit.ci to apply it as pre-commit.ci hasn't been turned on yet since these hooks were introduced in PR 21583.

Also remove comments in lib/matplotlib/backend_tools.py per request of maintainer in PR review.

Add a git blame ignore-revs file to automatically ignore commits listed in it from the blame view on GitHub. Include in it style changes added by pre-commit hooks which touch multiple files. To ignore the commits in the .git-blame-ignore-revs when running `git blame` locally run: `git blame --ignore-revs-file .git-blame-ignore-revs`. c.f.: * https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt * https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view

Add instructions to the devs on how to manually run pre-commit hooks, both for all the hooks, and for individual hook ids. Example: `pre-commit run flake8 --all-files` Also use the 'python -m pip' pattern to be consistent with the rest of the docs.

matthewfeickert · 2022-04-13T23:18:28Z

.git-blame-ignore-revs

+# style: end-of-file-fixer pre-commit hook
+c1a33a481b9c2df605bcb9bef9c19fe65c3dac21
+
+# style: trailing-whitespace pre-commit hook
+213061c0804530d04bbbd5c259f10dc8504e5b2b
+
+# style: check-docstring-first pre-commit hook
+046533797725293dfc2a6edb9f536b25f08aa636


After this last rebase:

$ git log --graph --oneline --decorate --all | head -n 7 * 48ade7d904 (HEAD -> mnt/apply-pre-commit-to-all-files, origin/mnt/apply-pre-commit-to-all-files) DOC: Add pre-commit hook run dev instructions * 4c003775bb DEV: Add .git-blame-ignore-revs to ignore style in blame view * 0465337977 STY: Apply check-docstring-first pre-commit hook * 213061c080 STY: Apply trailing-whitespace pre-commit hook * c1a33a481b STY: Apply end-of-file-fixer pre-commit hook * 787bdf08e5 MNT: Update pre-commit hooks to latest versions * 95038d96a4 DEV: Exclude LICENSE files from pre-commit

and

$ git grep "c1a33a481b\|0465337977\|213061c080" .git-blame-ignore-revs:c1a33a481b9c2df605bcb9bef9c19fe65c3dac21 .git-blame-ignore-revs:213061c0804530d04bbbd5c259f10dc8504e5b2b .git-blame-ignore-revs:046533797725293dfc2a6edb9f536b25f08aa636

so these are correct. 👍

As a single line checker:

$ git log --oneline | head -n 7 | grep STY | awk '{print $1}' | xargs -I {} git grep {} .git-blame-ignore-revs:046533797725293dfc2a6edb9f536b25f08aa636 .git-blame-ignore-revs:213061c0804530d04bbbd5c259f10dc8504e5b2b .git-blame-ignore-revs:c1a33a481b9c2df605bcb9bef9c19fe65c3dac21

matthewfeickert · 2022-04-13T23:20:31Z

@tacaswell The LICENSE/ dir is properly excluded now:

$ pre-commit run --all-files
check for added large files..............................................Passed
check docstring is first.................................................Passed
fix end of files.........................................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
flake8...................................................................Passed
$ git diff upstream/main -- LICENSE/  # No diff
$

doc/api/lines_api.rst

oscargus · 2022-04-14T12:28:18Z

setting a good precedent

Fair enough!

tacaswell · 2022-04-18T23:21:07Z

Thanks @matthewfeickert !

matthewfeickert commented Apr 8, 2022

View reviewed changes

matthewfeickert mentioned this pull request Apr 8, 2022

Demo of what pre-commit.ci looks like for the matplotlib dev team matthewfeickert/matplotlib#9

Closed

matthewfeickert mentioned this pull request Apr 9, 2022

DEV: Add codespell to pre-commit hooks #22777

Merged

jklymak approved these changes Apr 9, 2022

View reviewed changes

greglucas approved these changes Apr 9, 2022

View reviewed changes

matthewfeickert force-pushed the mnt/apply-pre-commit-to-all-files branch from c677636 to 02fc061 Compare April 9, 2022 16:02

matthewfeickert requested a review from greglucas April 9, 2022 16:05

oscargus reviewed Apr 10, 2022

View reviewed changes

tacaswell added this to the v3.6.0 milestone Apr 13, 2022

tacaswell approved these changes Apr 13, 2022

View reviewed changes

matthewfeickert added 7 commits April 13, 2022 18:06

DEV: Exclude LICENSE files from pre-commit

95038d9

Requested by Thomas Caswell in PR 22809

STY: Apply end-of-file-fixer pre-commit hook

c1a33a4

STY: Apply trailing-whitespace pre-commit hook

213061c

STY: Apply check-docstring-first pre-commit hook

0465337

Also remove comments in lib/matplotlib/backend_tools.py per request of maintainer in PR review.

matthewfeickert force-pushed the mnt/apply-pre-commit-to-all-files branch from 02fc061 to 48ade7d Compare April 13, 2022 23:15

matthewfeickert requested a review from tacaswell April 13, 2022 23:15

matthewfeickert commented Apr 13, 2022

View reviewed changes

QuLogic reviewed Apr 14, 2022

View reviewed changes

doc/api/lines_api.rst Show resolved Hide resolved

tacaswell merged commit f569f21 into matplotlib:main Apr 18, 2022

matthewfeickert deleted the mnt/apply-pre-commit-to-all-files branch April 18, 2022 23:40

matthewfeickert mentioned this pull request Apr 30, 2022

DEV: Decide if pre-commit should continue to be used #22949

Closed

matthewfeickert mentioned this pull request Oct 17, 2022

MNT: Update pre-commit hooks #24196

Merged

1 task

matthewfeickert mentioned this pull request May 29, 2024

ci: Add pre-commit thaler-lab/Wasserstein#12

Merged

matthewfeickert mentioned this pull request Jan 17, 2025

Full changes from pre-commit and black ssl-hep/ServiceX_frontend#545

Merged

matthewfeickert mentioned this pull request Mar 11, 2025

Migrate to src directory layout snakemake/snakemake#3371

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STY: Apply pre-commit hooks to codebase #22809

STY: Apply pre-commit hooks to codebase #22809

matthewfeickert commented Apr 8, 2022 •

edited

Loading

matthewfeickert Apr 8, 2022

jklymak Apr 9, 2022

matthewfeickert Apr 9, 2022

matthewfeickert Apr 8, 2022

matthewfeickert commented Apr 8, 2022 •

edited

Loading

matthewfeickert commented Apr 8, 2022 •

edited

Loading

oscargus commented Apr 9, 2022

matthewfeickert commented Apr 9, 2022

jklymak left a comment

oscargus commented Apr 9, 2022

greglucas left a comment

greglucas Apr 9, 2022

matthewfeickert Apr 9, 2022

matthewfeickert commented Apr 9, 2022

oscargus Apr 10, 2022

matthewfeickert Apr 11, 2022 •

edited

Loading

oscargus Apr 14, 2022

oscargus Apr 14, 2022

matthewfeickert Apr 14, 2022 •

edited

Loading

henryiii Apr 15, 2022

QuLogic Apr 15, 2022

matthewfeickert Apr 15, 2022

greglucas Apr 15, 2022

matthewfeickert Apr 18, 2022

oscargus commented Apr 10, 2022

greglucas commented Apr 10, 2022

matthewfeickert commented Apr 11, 2022

tacaswell left a comment

matthewfeickert commented Apr 13, 2022 •

edited

Loading

matthewfeickert Apr 13, 2022 •

edited

Loading

matthewfeickert commented Apr 13, 2022

oscargus commented Apr 14, 2022

tacaswell commented Apr 18, 2022

STY: Apply pre-commit hooks to codebase #22809

STY: Apply pre-commit hooks to codebase #22809

Conversation

matthewfeickert commented Apr 8, 2022 • edited Loading

PR Summary

PR Checklist

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

matthewfeickert commented Apr 8, 2022 • edited Loading

matthewfeickert commented Apr 8, 2022 • edited Loading

oscargus commented Apr 9, 2022

matthewfeickert commented Apr 9, 2022

jklymak left a comment

Choose a reason for hiding this comment

oscargus commented Apr 9, 2022

greglucas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewfeickert commented Apr 9, 2022

Choose a reason for hiding this comment

matthewfeickert Apr 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewfeickert Apr 14, 2022 • 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

oscargus commented Apr 10, 2022

greglucas commented Apr 10, 2022

matthewfeickert commented Apr 11, 2022

tacaswell left a comment

Choose a reason for hiding this comment

matthewfeickert commented Apr 13, 2022 • edited Loading

matthewfeickert Apr 13, 2022 • edited Loading

Choose a reason for hiding this comment

matthewfeickert commented Apr 13, 2022

oscargus commented Apr 14, 2022

tacaswell commented Apr 18, 2022

matthewfeickert commented Apr 8, 2022 •

edited

Loading

matthewfeickert commented Apr 8, 2022 •

edited

Loading

matthewfeickert commented Apr 8, 2022 •

edited

Loading

matthewfeickert Apr 11, 2022 •

edited

Loading

matthewfeickert Apr 14, 2022 •

edited

Loading

matthewfeickert commented Apr 13, 2022 •

edited

Loading

matthewfeickert Apr 13, 2022 •

edited

Loading