Skip to content

Add ruff formatter - #60

Merged
reweeden merged 7 commits into
mainfrom
rew/add-ruff-formatter
Oct 13, 2025
Merged

Add ruff formatter#60
reweeden merged 7 commits into
mainfrom
rew/add-ruff-formatter

Conversation

@reweeden

@reweeden reweeden commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

Based on #59

Uses the config from the ia-standards repo https://github.com/asfadmin/ia-standards/blob/main/pyproject.toml.

Pull Request Checklist

I have:

  • performed a self review of my code I&A code style
    • Resources and Data Structures are sorted by ABC or a defined sorting pattern
  • updated the documentation accordingly
  • verified required action checks are passing
  • bumped the version number as appropriate

@reweeden
reweeden force-pushed the rew/add-ruff-formatter branch 2 times, most recently from 8a4e211 to 3343c62 Compare August 12, 2025 16:49
@reweeden
reweeden marked this pull request as ready for review August 12, 2025 16:55
@reweeden reweeden mentioned this pull request Aug 12, 2025
Closed
4 tasks
Comment thread destroy-cumulus/destroy_cumulus.py Outdated

@mckadesorensen mckadesorensen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please close this PR, as you know, I already have one open. I will address issues in that PR as I see fit. In the future, please do not open a PR when I openly have one open on the topic.

@reweeden

Copy link
Copy Markdown
Contributor Author

Please close this PR, as you know, I already have one open. I will address issues in that PR as I see fit. In the future, please do not open a PR when I openly have one open on the topic.

Hey man, I mean no offense. I literally made this as a courtesy to you because I know that you don't like comments about formatting on your PRs, and I figured I would put in the work myself as a gesture of good faith collaboration. But by your comment it seems it was not received that way. So I apologize, I really am just trying to help find a middle ground that suits both of us.

From what I recall you told me that you don't care how the code is formatted as long as it passes the ruff formatter check. The code here is formatted with ruff, so this should suit your needs exactly.

@reweeden
reweeden force-pushed the rew/add-ruff-formatter branch 5 times, most recently from 8769809 to f6899fe Compare August 13, 2025 16:18
@reweeden
reweeden requested a review from gjclark August 13, 2025 16:20
@mckadesorensen

Copy link
Copy Markdown
Contributor

Please close this PR, as you know, I already have one open. I will address issues in that PR as I see fit. In the future, please do not open a PR when I openly have one open on the topic.

Hey man, I mean no offense. I literally made this as a courtesy to you because I know that you don't like comments about formatting on your PRs, and I figured I would put in the work myself as a gesture of good faith collaboration. But by your comment it seems it was not received that way. So I apologize, I really am just trying to help find a middle ground that suits both of us.

From what I recall you told me that you don't care how the code is formatted as long as it passes the ruff formatter check. The code here is formatted with ruff, so this should suit your needs exactly.

While I don't care how the code is formatted as long as it passes the ruff lint tests, I also don't want to have to think about adding magic commas in the future. My concern is that in the future, you will say that this repo is formatted using the magic comma, and to get your approval, we will need to be "consistent." The purpose is to remove opinions from formatting. I don't like everything Ruff does, but I do like the fact that it removes opinions from the conversation.

@reweeden

reweeden commented Aug 13, 2025

Copy link
Copy Markdown
Contributor Author

I see. It was my impression that going forward we would be operating under the rule that anything goes as long as the ruff auto-formatter agrees with it. That's why I want to make sure that's explicitly stated in our coding standards and have suggested language to that extent in this PR: https://github.com/asfadmin/ia-standards/pull/12

You are right that there will almost certainly be times when a PR is made that is formatted by ruff in a way that is different from how I would do it. My thought is that given the above ^^ the changes would be accepted, and if it's something that really bothers me later, I can always tweak it by adding magic commas or whatever myself so long as the formatting still agrees with ruff. But I would not be commenting those things on the PR as long as the ruff auto-formatter github action passed. This to me seems like a win-win compromise that satisfies both of our needs.

That said, I commented these things on your PR specifically because it is a reformatting PR where we're reformatting the entire code base and adding ruff for the first time, and in my mind it would be best to start from a baseline where everyone's needs are met (rather than you making a PR and me then making a separate PR to add magic commas, etc). I also figured (correctly) that you would have no interest in making such changes yourself, which is why I made this PR in an attempt to save us from the back and forth, and save us a step by just doing it all in one.

Comment thread .github/workflows/lint.yml
@reweeden
reweeden requested a review from mattp0 September 3, 2025 15:48
@reweeden
reweeden force-pushed the rew/add-ruff-formatter branch 3 times, most recently from 7631f42 to 56af6d7 Compare September 26, 2025 16:03
@reweeden
reweeden requested review from mattp0 and removed request for gjclark and mattp0 September 29, 2025 15:21
@reweeden
reweeden requested a review from gjclark September 29, 2025 15:21
@reweeden
reweeden force-pushed the rew/add-ruff-formatter branch 3 times, most recently from 9cd87f1 to a0621f7 Compare October 3, 2025 15:51
@reweeden
reweeden force-pushed the rew/add-ruff-formatter branch from a0621f7 to a78332d Compare October 3, 2025 15:59
- uses: actions/checkout@v4
- uses: astral-sh/ruff-action@v3
with:
version: "~=0.13.3"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version adds support for the --output-format flag for the formatter which would be nice to use the github output format. However, it's only available with --preview which also enables preview formatting features. I actually like the preview formatting better but it would probably be best to discuss that and make it official before enabling it, so I'm gonna hold off on that. Hopefully the --output-format is stabalized soon, for now it just emits a warning.

https://github.com/astral-sh/ruff/releases/tag/0.13.3

ruff format --check --diff --output-format=github
warning: The --output-format flag for the formatter is unstable and requires preview mode to use.
67 files already formatted

@reweeden
reweeden merged commit 9633e3a into main Oct 13, 2025
6 checks passed
@reweeden
reweeden deleted the rew/add-ruff-formatter branch October 13, 2025 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants