Fix linting issue

[jcadam14]

Linting with black is failing on a file that was merged into main even though it did NOT fail on the PR.

[michaeljwood]

I believe the linting issue was fixed with #292 , but I realized that the linter is not configured to run on PRs linters.yml, and because I was working on a fork, the action did not run on push to my fork.

I admit I could/should have caught this locally, but should probably make sure that runs on PRs (to main at least). I believe the sbl-filing-api at least also has this potential issue.

[jcadam14]

I believe the linting issue was fixed with #292 , but I realized that the linter is not configured to run on PRs linters.yml, and because I was working on a fork, the action did not run on push to my fork.

I admit I could/should have caught this locally, but should probably make sure that runs on PRs (to main at least). I believe the sbl-filing-api at least also has this potential issue.

Linters used to run on all our PRs, very common to have the checks fail due to someone not running black and ruff locally before pushing up (at least in the past, we've gotten pretty good about that now). So yeah I'm a little baffled why that stopped. I'll dig and figure out what broke. But linting was a thing, now it appears it's not in our repos.

[jcadam14]

Going to update this story to include updating the linter.yaml with

on:
  pull_request:
  push:
    branches:
      - "main"

[michaeljwood]

Didn't notice your comment until just now. Did you think it was important to run when pushing to main only? I had pushed a change to add the PR bit, but left it as pushes to any branch.

[jcadam14]

Didn't notice your comment until just now. Did you think it was important to run when pushing to main only? I had pushed a change to add the PR bit, but left it as pushes to any branch.

Personally I dislike when I'm on a prototype/test/sandbox branch where I'm just playing around with different approaches, getting emails that my branch failed linting because during that phase I don't really care about it. Only when I'm ready to make the code official do I think linting needs to be considered. That's my personal preference. @lchen-2101 thoughts? Push to main or just any push?

[jcadam14]

Looking more at the other repos, we are getting linting running on other PRs, so must be something with the way you used a fork perhaps, not sure. Probably better to explicitly call it out like we do with tests

[lchen-2101]

just to main sounds good to me, not a fan of getting notified on all the branches; and yeah, we haven't done much with forks, so new territory for us.

[michaeljwood]

Although you can just disable actions on the fork, I think my issue was more that I just didn't notice the failure because it was not noisy and did not stop me from opening the PR, so I just missed it.