Skip to content

Conversation

grahamalama
Copy link
Contributor

Checklist:

Before submitting your PR, please confirm that you have done the following:

  • I have opened my PR against the staging branch, NOT against main
  • I've run the relevant formatting and linting tools listed in the setup docs
  • I have commented hard-to-understand areas in my code
  • I've reviewed any merge conflicts to make sure they are resolved
  • My changes generate no new warnings
    • I noticed some pre-commit issues when I tried submitting the formatting fixes, but I'm hoping that was an issue with my local development environment and not with the actual changes present here

Description

I noticed that the formatting check used in CI was lacking the --check flag for ruff. This meant that ruff formatted the files and exited with 0, even if there were files left to be formatted. Since this is used as a pre-commit hook where we do want to maintain automatic formatting, I opted to only override the command we pass to the formatter container in Github actions. We may want to rework CI checks in the future. But this at least ensures we catch formatting errors in the short term.

Type of change

  • Bug fix
  • New feature
  • Breaking change

How Has This Been Tested?

Since these were only formatting changes, I didn't add any new tests.

Copy link

vercel bot commented Apr 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
vacant-lots-proj ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 15, 2025 8:46pm

@cfreedman
Copy link
Contributor

Thanks for the catch, yea this makes sense to me.

@cfreedman cfreedman merged commit d98f7dc into CodeForPhilly:staging Apr 16, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants