dify icon indicating copy to clipboard operation
dify copied to clipboard

automate isort styling by adding to pre-commit hooks

Open bowenliang123 opened this issue 1 year ago • 3 comments

  • as #1983 introduced isort and one-key linting scripts for linting Python imports, it helps to conform a steady output in sorted imports within a unified style.
  • this PR add the isort script into Husky for git's pre-commit hooks, which is to be executed automatically before each commit
  • This will greatly help the developer out of the dirty work when rebasing onto the new release and branches without worrying the conflict in Python imports.

bowenliang123 avatar Feb 01 '24 06:02 bowenliang123

Hi @takatost , this PR has been rebased onto latest commit, and it's ready to go.

bowenliang123 avatar Feb 01 '24 10:02 bowenliang123

What happens if the developer only modifies the frontend code without having the Python environment or without installing isort? In that case, the pre-commit hook won't be able to run, I think.

takatost avatar Feb 01 '24 11:02 takatost

#!/usr/bin/env sh

# get the list of modified files
files=$(git diff --cached --name-only)

api_modified=false
web_modified=false

for file in $files
do
    if [[ $file == "api/"* && $file == *.py ]]; then
        # set api_modified flag to true
        api_modified=true
    elif [[ $file == "web/"* ]]; then
        # set web_modified flag to true
        web_modified=true
    fi
done

# check if api or web directory is modified
if $api_modified; then
    echo "do something for api"
fi

if $web_modified; then
    echo "do something for web"
fi

This script can determine what subsequent actions need to be taken by retrieving file modification records. For example, if the file is located in the "api" directory and has a .py extension, it will execute the "reformat" command.

takatost avatar Feb 01 '24 11:02 takatost

why not bring in black as well?

wlbksy avatar Feb 02 '24 10:02 wlbksy

Several popular Python linter tools were considered, including black, isort, ruff and etc. As dify community requested minimum impact to existed code for commit history traceability, isort is introduced to sort and refomrated the Python's imports solely.

In the future, I would prefer introduce the ruff for general Python linter for compatible for Flake8,black and isort. And ruff is extremely fast which is implemented in Rust.

bowenliang123 avatar Feb 02 '24 13:02 bowenliang123

Closing this PR. ruff seems to to be a much better option for lining Python imports. As we want to apply Python imports linting in pre-commit stage, we'd better choose some tools like ruff does. It's extremely fast.

bowenliang123 avatar Feb 02 '24 16:02 bowenliang123