Add a pre-commit hook for running black and pylint

[harupy]

Signed-off-by: harupy hkawamura0130@gmail.com

What changes are proposed in this pull request?

Add a pre-commit hook for running black and pylint to avoid re-running CI just to fix lint errors.

How is this patch tested?

• Create a file that contains a lint error.
• Make a commit to ensure the pre-commit hook aborts it.

% git diff
diff --git a/dev/show_package_release_dates.py b/dev/show_package_release_dates.py
index 76b653792..44792ba9c 100644
--- a/dev/show_package_release_dates.py
+++ b/dev/show_package_release_dates.py
@@ -6,7 +6,7 @@ import traceback
 
 
 def get_distributions():
-    res = subprocess.run(["pip", "list"], stdout=subprocess.PIPE, check=True)
+    res = subprocess.run(["pip", "list"], stdout=subprocess.PIPE)
     pip_list_stdout = res.stdout.decode("utf-8")
     # `pip_list_stdout` looks like this:
     # ``````````````````````````````````````````````````````````
                                                                                              
% git add .
                                                                                              
% git commit --signoff -m test
+ git diff --cached --name-only
+ grep \.py$
+ diff=dev/show_package_release_dates.py
+ [ ! -z dev/show_package_release_dates.py ]
+ black --check dev/show_package_release_dates.py
All done! ✨ 🍰 ✨
1 file would be left unchanged.
+ pylint dev/show_package_release_dates.py
************* Module show_package_release_dates
dev/show_package_release_dates.py:9:10: W1510: Using subprocess.run without explicitly set `check` is not recommended. (subprocess-run-check)

------------------------------------------------------------------
Your code has been rated at 9.76/10 (previous run: 9.76/10, +0.00)

Release Notes

Is this a user-facing change?

• No. You can skip the rest of this section.
• Yes. Give a description of this change to be included in the release notes for MLflow users.

(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)

What component(s), interfaces, languages, and integrations does this PR affect?

Components

• area/artifacts: Artifact stores and artifact logging
• area/build: Build and test infrastructure for MLflow
• area/docs: MLflow documentation pages
• area/examples: Example code
• area/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registry
• area/models: MLmodel format, model serialization/deserialization, flavors
• area/projects: MLproject format, project running backends
• area/scoring: MLflow Model server, model deployment tools, Spark UDFs
• area/server-infra: MLflow Tracking server backend
• area/tracking: Tracking Service, tracking client APIs, autologging

Interface

• area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
• area/docker: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
• area/sqlalchemy: Use of SQLAlchemy in the Tracking Service or Model Registry
• area/windows: Windows support

Language

• language/r: R APIs and clients
• language/java: Java APIs and clients
• language/new: Proposals for new client languages

Integrations

• integrations/azure: Azure and Azure ML integrations
• integrations/sagemaker: SageMaker integrations
• integrations/databricks: Databricks integrations

How should the PR be classified in the release notes? Choose one:

• rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
• rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
• rn/feature - A new user-facing feature worth mentioning in the release notes
• rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
• rn/documentation - A user-facing documentation change worth mentioning in the release notes

[harupy]

Only check the changed files to save the execution time.

[harupy]

This hook can be enabled via:

git config core.hooksPath hooks

[liangz1]

It would be nice to also update https://github.com/mlflow/mlflow/blob/master/CONTRIBUTING.rst#common-prerequisites-and-dependencies

For convenience, we provide a pre-commit git hook that validates that commits are signed-off.

to

For convenience, we provide a pre-commit git hook that validates that commits are signed-off, auto-formats the code, and runs pylint.

[harupy]

Sounds good!

[harupy]

Updated!

[liangz1]

Only one question about the file name:
Currently, the two files in hooks/ are named as pre-commit and prepare-commit-msg, which are not directly reflecting their functions. Shall we rename them to autoformat-and-check-lint and check-sign-off? Or something similar?

[harupy]

Only one question about the file name:
Currently, the two files in hooks/ are named as pre-commit and prepare-commit-msg, which are not directly reflecting their functions. Shall we rename them to autoformat-and-check-lint and check-sign-off? Or something similar?

@liangz1

Good point, but pre-commit and prepare-commit-msg are special file names so we can't change them.

[liangz1]

LGTM. Thanks!