Skip to content

Python code format #1954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 27, 2025
Merged

Conversation

MonkeyCanCode
Copy link
Contributor

@MonkeyCanCode MonkeyCanCode commented Jun 26, 2025

Formatted python code base with ruff (https://docs.astral.sh/ruff/). If there is no concern with this route, should we consider use ruff for code formatting and reject PRs that doesn't follow the format (same mechanism is there for current java code base via RAT). Ruff is under MIT license.

@MonkeyCanCode
Copy link
Contributor Author

MonkeyCanCode commented Jun 27, 2025

@snazy @jbonofre maybe get your input here for the code formatting automation via CI on the PR via ruff.

@eric-maynard
Copy link
Contributor

Ideally, we should add this to CI so that ./gradlew test surfaces these issues before CI has to run

@MonkeyCanCode
Copy link
Contributor Author

Ideally, we should add this to CI so that ./gradlew test surfaces these issues before CI has to run

Yes, that would be nice I think. let me see how can I link gradlew with ruff or is there is other alternatives that we can use.

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jun 27, 2025
@MonkeyCanCode MonkeyCanCode merged commit 8784135 into apache:main Jun 27, 2025
11 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Jun 27, 2025
@MonkeyCanCode
Copy link
Contributor Author

Thanks for review @eric-maynard. I will open another one once I have the automation ready based on above discussion.

@eric-maynard
Copy link
Contributor

eric-maynard commented Jul 2, 2025

@MonkeyCanCode what exactly did you run here btw? I still see conflicts/changes on #1912 after running ruff check client/python/cli --fix there.

@MonkeyCanCode
Copy link
Contributor Author

MonkeyCanCode commented Jul 2, 2025

@MonkeyCanCode what exactly did you run here btw? I still see conflicts/changes on #1912 after running ruff check client/python/cli --fix there.

For the above, I am using uv to setup ruff as ruff is not part of the current tool we used. You can do the same with following instructions:

➜  polaris git:(main) cd client
➜  client git:(main) uv add --dev ruff
➜  client git:(main) uv run ruff format

I will look into on how we can automate this tomorrow.

@MonkeyCanCode
Copy link
Contributor Author

@MonkeyCanCode what exactly did you run here btw? I still see conflicts/changes on #1912 after running ruff check client/python/cli --fix there.

For the above, I am using uv to setup ruff as ruff is not part of the current tool we used. You can do the same with following instructions:

➜  polaris git:(main) cd client
➜  client git:(main) uv add --dev ruff
➜  client git:(main) uv run ruff format

I will look into on how we can automate this tomorrow.

@eric-maynard sample PR is ready: #1986

let me know what you think. Thanks.

@MonkeyCanCode
Copy link
Contributor Author

@MonkeyCanCode what exactly did you run here btw? I still see conflicts/changes on #1912 after running ruff check client/python/cli --fix there.

For the above, I am using uv to setup ruff as ruff is not part of the current tool we used. You can do the same with following instructions:

➜  polaris git:(main) cd client
➜  client git:(main) uv add --dev ruff
➜  client git:(main) uv run ruff format

I will look into on how we can automate this tomorrow.

@eric-maynard sample PR is ready: #1986

let me know what you think. Thanks.

@eric-maynard after #1995 is merged, u can just run make lint for this to work without tedious instructions above.

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.

2 participants