Skip to content
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

Implement automatic and uniform coding standard #170

Closed
timcera opened this issue Jul 11, 2024 · 5 comments
Closed

Implement automatic and uniform coding standard #170

timcera opened this issue Jul 11, 2024 · 5 comments

Comments

@timcera
Copy link
Contributor

timcera commented Jul 11, 2024

There are several ways to reduce technical debt for a programming project and one way is to have a uniform coding standard. I propose that we adopt "ruff format ." and "ruff check --fix ." along with "pre-commit" for HSPsquared. Since this would reformat every file posting this as an issue for discussion and to plan the implementation.

The ruff home page is https://docs.astral.sh/ruff/ and there are several ways to install (https://docs.astral.sh/ruff/installation/)

There are ruff extensions for VS Code and I am sure other IDEs that will run ruff on save so that you can reformat as you go, however I am also suggesting using "pre-commit" which will run ruff when you use git to commit the code.

"pre-commit" installs git hooks that test all files in the commit and will run ruff and other reformatters and linters before letting git to actually process the commit. The "pre-commit" home page is https://pre-commit.com/ for more information.

A ".pre-commit-config.yaml" is added to the top level of the project where we can configure all aspects of the code standard that we want to implement including running ruff. I suggest a good starting point would be my pre-commit configuration at https://github.com/timcera/tsblender/blob/main/.pre-commit-config.yaml.

The best approach is to apply this is all at once, which means there would be a single commit that would just be restyling the code. After that then using pre-commit will only reformat edited or new code to the accepted standard.

@PaulDudaRESPEC
Copy link
Member

Hey @timcera , thanks for doing this! I'm very interested in using this system to enforce a standard -- does anyone else in our development community want to chime in???

Does ruff do roughly the same thing as black?

@timcera
Copy link
Contributor Author

timcera commented Jul 31, 2024

Ruff as an autoformatter

"ruff format filename.py" == "black filename.py"

Ruff as a linter

"ruff check filename.py" is a linter - typically writing out a report and not changing anything, but "ruff check --fix filename.py" will change the file where ruff determined that it can implement safe fixes. An example of a safe fix would be removing variables that are set to some value, but aren't used. I turned off the "ruff check --fix filename.py" in the .pre-commit-config.yaml for the time being, but should be turned back on - however lots of work to fix all the problems that "ruff check ..." finds. In commit #174 I included all of "ruff check ..." safe fixes.

@PaulDudaRESPEC
Copy link
Member

@timcera , I took at good look at the modified code, and I'm pleased with it. While there are a lot of changes, the code is still very familiar and I don't see any problems adopting this as a standard.

One question -- I see your PR to merge into the main... is there any reason this couldn't be merged into the 'develop' branch first? Just thinking that's been more our SOP until we're ready for a new release.

@timcera
Copy link
Contributor Author

timcera commented Aug 7, 2024

The pull request should have been into develop. Apologies. I intended to follow your normal workflow.

@aufdenkampe
Copy link
Collaborator

aufdenkampe commented Dec 18, 2024

I love that this was implemented!

Closing this, given that the following PR was merged:

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

No branches or pull requests

3 participants