godot-demo-projects icon indicating copy to clipboard operation
godot-demo-projects copied to clipboard

Add a .pre-commit-hooks.yaml and add it to contributing guideline

Open m21-cerutti opened this issue 1 year ago • 3 comments

Which demo project is affected: All possibly to clean.

Description: Could be interesting to add a pre commit hook to clean commits before pull requests and add it to recommended guidelines. Would make smoother I think the review process.

Resources: https://pre-commit.com/ https://github.com/Scony/godot-gdscript-toolkit/blob/master/.pre-commit-hooks.yaml

For context I got the idea with this review https://github.com/godotengine/godot-demo-projects/pull/1085

m21-cerutti avatar Jul 13 '24 09:07 m21-cerutti

This is a good idea, yeah. We already have this for the engine repo, having pre-commit here too would be nice. At a minimum it could perform the same basic checks as CI, at best it could perform more formatting.

aaronfranke avatar Jul 13 '24 09:07 aaronfranke

I tried to have a first try with

default_install_hook_types: [pre-commit]
repos:

# Linters
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.5.0
    hooks:
    -   id: check-added-large-files
    -   id: check-case-conflict
    -   id: check-merge-conflict
    -   id: end-of-file-fixer
    -   id: trailing-whitespace
    -   id: check-json
    -   id: check-symlinks

-   repo: https://github.com/Scony/godot-gdscript-toolkit
    rev: 4.2.2
    hooks:
        - id: gdlint
          exclude: .*screenshots/
        - id: gdformat
          exclude: .*screenshots/

but seems it would need more setup

image

m21-cerutti avatar Jul 13 '24 09:07 m21-cerutti

Note that, as seen in https://github.com/godotengine/godot-demo-projects/pull/1085, the proposed linter does not correctly follow the GDScript style guide, so it'd have to be improved to properly handle that before it can be used here

AThousandShips avatar Jul 13 '24 11:07 AThousandShips