yoheimuta / protolint

A pluggable linter and fixer to enforce Protocol Buffer style and conventions.
MIT License
577 stars 52 forks source link

Configuration not loading, issues with pre-commit? #281

Closed jenstroeger closed 2 years ago

jenstroeger commented 2 years ago

Hello, thanks for the tool which works fine when I use it standalone from the command-line:

> protolint lint protos/
# Heaps of messages

However, trying to run this from pre-commit using the suggested settings gives

> pre-commit run protolint 
Lint Protocol Buffer Files...........................(no files to check)Skipped

Adding to the .pre-commit-config.yaml the following:

   - id: protolint
     args: [protos/]

or

  - id: protolint
    files: ^protos/

didn’t fix the issue, and neither did

  - id: protolint
    args: [--config_path, .protolint.yaml]

where I listed the protos/ folder:

lint:
  directories:
  - 'protos/'

The .protolint.yaml configuration file doesn’t seem to be loaded, because it actually fails:

> protolint lint --config_path .protolint.yaml protos/
yaml: unmarshal errors:
  line 4: cannot unmarshal !!seq into config.Directories

So, I’m a bit confused about configuring protolint here 🤔

yoheimuta commented 2 years ago

@jenstroeger

Lint Protocol Buffer Files...........................(no files to check)Skipped

Do you have any changes to be committed regarding proto files?

it's usually a good idea to run the hooks against all of the files when adding new hooks (usually pre-commit will only run on the changed files during git hooks)

https://pre-commit.com/#using-the-latest-version-for-a-repository

How about pre-commit run -a protolint?

jenstroeger commented 2 years ago

Do you have any changes to be committed regarding proto files?

Yes, files in the protos/ folder are on git’s staged area:

> git status
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
    ...
    modified:   protos/v1/test.proto
    ...

How about pre-commit run -a protolint?

Same:

> pre-commit run -a protolint
Lint Protocol Buffer Files...........................(no files to check)Skipped

And if it helps:

> pre-commit --version
pre-commit 2.20.0

The meta hook also suggests that the protolint hook doesn’t apply to the repo, which is odd because the repo does contain *.proto files in a subfolder:

Check hooks apply to the repository......................................Failed
- hook id: check-hooks-apply
- exit code: 1

protolint does not apply to this repository
yoheimuta commented 2 years ago

@jenstroeger Can you give me the entire minimum.pre-commit-config.yaml to reproduce it? Just in case.

# I used this yaml.
(base) ❯ cat .pre-commit-config.yaml
repos:
  - repo: https://github.com/yoheimuta/protolint
    rev: master
    hooks:
      - id: protolint
jenstroeger commented 2 years ago

Sigh. All my fault 🤦🏻‍♂️

When I dumped the .pre-commit-config.yaml

# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
default_install_hook_types: [pre-commit, commit-msg, pre-push]
default_stages: [commit]
exclude: ^Makefile|^protos/
minimum_pre_commit_version: 2.18.0

repos:
- repo: meta
  hooks:
  - id: check-hooks-apply
  - id: check-useless-excludes
- repo: https://github.com/yoheimuta/protolint
  rev: v0.39.0
  hooks:
  - id: protolint
...

Notice the top-level exclude there. Feel free to close this issue!

yoheimuta commented 2 years ago

Got it 😸