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

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

Open
m21-cerutti opened this issue Jul 13, 2024 · 3 comments
Open

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

m21-cerutti opened this issue Jul 13, 2024 · 3 comments

Comments

@m21-cerutti
Copy link

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
#1085

@aaronfranke
Copy link
Member

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.

@m21-cerutti
Copy link
Author

m21-cerutti commented Jul 13, 2024

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

@AThousandShips
Copy link
Member

Note that, as seen in #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants