Validate and implement Linter before making a commit

[airvzxf]

Validate and implement Linter before making a commit.

Description

I recommend that we implement and use Git Hooks (Atlassian and Geeks for Geeks), which allow us to validate and format the code when making a local commit, among many other things. Therefore, when we make a push to the server, it will already be validated and should pass the “code style / black (pull_request) workflow”. I implemented it a few years ago, and it was useful.

Proposal

I saw two ways to implement them, and I'll describe their pros and cons.

1. Implement the “pre-commit” on the client-side hooks.
   • Pro: Every local commit will be formatted before being committed.
   • Con: If the academic or software developer is making small commits and there are many of them, it can be annoying for them.
2. Or implement the “pre-receive” on the server-side hooks.
   • Pro: Make all the formats at once, preventing uploading the changes to GitHub if they are not formatted.
   • Cons: If there were additional modifications, this commit will contain plenty of format changes.

Types of Hooks:

Some other considerations, is that the academic or software developer needs to install black for any commit, but we can configure to detect if the changes were in the code (tardis folder) and apply only for them.

[andrewfullard]

We actually already have these, implemented in #2509

[airvzxf]

I was reviewing the implementation in #2509. I have mixed feelings because formatting is an optional installation with the pre-commit package. However, it is mandatory to apply formatting; otherwise, the code does not pass the code style workflow. In other words, in the end, formatting the code is mandatory. Therefore, it can be done from the moment the commit is uploaded, and not after your commits are uploaded to the GitHub server.

I saw some pull requests, especially in GSoC, in which the new contributors didn't run black or format the code as TARDIS is expected in the code style workflow. Furthermore, I am one which has some pull requests with no achieve the expectations for the format.

If my arguments were sufficiently supported and TARDIS decided to resolve this situation, I could recommend two things separately:

1. The “pre-commit” package went from being optional to being installed automatically when running the TARDIS setup with Conda.
2. Or, implement the basic “pre-commit” Git Hook (.git/hooks/pre-commit) with black and if someone decides to install the “pre-commit”, then it will replace black.

How do you see this situation? What do you think about it?

[airvzxf]

Description

I am having a bad experience as a user. I installed pre-commit and used it in my TARDIS project because I wanted to correct a black error in my last GitHub commits. pre-commit shows more errors than black but most importantly, pre-commit is not showing the black error. It means if I fix the pre-commit errors, then my commit on GitHub will be failing forever. It was some of my concerns that I wrote about (or thought) for the implementation in #2509.

Suggest

If the filter in GitHub Actions is black, the most recommended implementation in the local Git pre-commit is to use black.

---

Evidence

This is the execution for pre-commit in my local:

pipenv run python -m pre_commit run --config=.pre-commit-config.yaml --files tardis/io/model/readers/blondin_toymodel.py

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
ruff.....................................................................Failed
- hook id: ruff
- exit code: 1

tardis/io/model/readers/blondin_toymodel.py:31:10: UP015 [*] Unnecessary open mode parameters
tardis/io/model/readers/blondin_toymodel.py:94:10: UP015 [*] Unnecessary open mode parameters
tardis/io/model/readers/blondin_toymodel.py:103:9: C408 Unnecessary `dict` call (rewrite as a literal)
tardis/io/model/readers/blondin_toymodel.py:110:9: C408 Unnecessary `dict` call (rewrite as a literal)
tardis/io/model/readers/blondin_toymodel.py:113:9: C408 Unnecessary `dict` call (rewrite as a literal)
tardis/io/model/readers/blondin_toymodel.py:118:13: C408 Unnecessary `dict` call (rewrite as a literal)
Found 6 errors.
[*] 2 fixable with the `--fix` option (4 hidden fixes can be enabled with the `--unsafe-fixes` option).

isort....................................................................Passed

The black output in my local is:

pipenv run black tardis/io/model/readers/blondin_toymodel.py --color --check

would reformat tardis/io/model/readers/blondin_toymodel.py

Oh no! 💥 💔 💥
1 file would be reformatted.

The problem in some extra comma which pre-commit is not detected.

pipenv run black tardis/io/model/readers/blondin_toymodel.py --color --check --diff                                                
--- tardis/io/model/readers/blondin_toymodel.py 2024-04-30 10:32:49.769562+00:00                                                                  
+++ tardis/io/model/readers/blondin_toymodel.py 2024-04-30 10:35:18.458664+00:00                                                                  
@@ -48,11 +48,11 @@                                                                                                                               
         #   The regular expression means: the separation is one or                                                                               
         #   more spaces together (simple space, tabs, new lines).                                                                                
         sep=r"\s+",                                                                                                                              
         comment="#",                                                                                                                             
         header=None,                                                                                                                             
-        names=columns                                                                                                                            
+        names=columns,                                                                                                                           
     )                                                                                                                                            

--

https://github.com/airvzxf/tardis/actions/runs/8893046433/job/24418431698

black --check tardis
# Skipping .ipynb files as Jupyter dependencies are not installed.
# You can fix this by running ``pip install black[jupyter]``
# would reformat tardis/io/model/readers/blondin_toymodel.py