[GitHub] [apisix] jbampton commented on a change in pull request #3624: feat: add pre-commit framework
jbampton commented on a change in pull request #3624: URL: https://github.com/apache/apisix/pull/3624#discussion_r627089487 ## File path: CONTRIBUTING.md ## @@ -51,9 +51,42 @@ Once we've discussed your changes and you've got your code ready, make sure that ## Contribution Guidelines for Documentation +* pre-commit + +A framework for managing and maintaining multi-language pre-commit hooks. +Pre-commit can be [installed](https://pre-commit.com/#installation) with `pip`, `curl`, `brew` or `conda`. +You need to first install pre-commit and then install the pre-commit hooks with `pre-commit install`. +Now pre-commit will run automatically on git commit! Review comment: Having as much information as possible on `pre-commit` is necessary. So far both you and @spacewander don't seem to understand much about `pre-commit`. Perhaps we need a new Markdown file for these docs ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] jbampton commented on a change in pull request #3624: feat: add pre-commit framework
jbampton commented on a change in pull request #3624: URL: https://github.com/apache/apisix/pull/3624#discussion_r627087054 ## File path: CONTRIBUTING.md ## @@ -51,9 +51,42 @@ Once we've discussed your changes and you've got your code ready, make sure that ## Contribution Guidelines for Documentation +* pre-commit + +A framework for managing and maintaining multi-language pre-commit hooks. +Pre-commit can be [installed](https://pre-commit.com/#installation) with `pip`, `curl`, `brew` or `conda`. Review comment: Most people would not install `pre-commit` with `curl` if they had the choice to use `pip`, `brew` or `conda`. I use Homebrew and I update my packages everyday and it makes it so much easier to use and manage. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] jbampton commented on a change in pull request #3624: feat: add pre-commit framework
jbampton commented on a change in pull request #3624: URL: https://github.com/apache/apisix/pull/3624#discussion_r627083105 ## File path: .pre-commit-config.yaml ## @@ -0,0 +1,193 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +--- +default_stages: [commit, push] +default_language_version: + # force all unspecified Python hooks to run python3 + python: python3 +minimum_pre_commit_version: "1.20.0" +repos: + - repo: meta +hooks: + - id: identity + - id: check-hooks-apply + - repo: https://github.com/Lucas-C/pre-commit-hooks +rev: v1.1.10 +hooks: + # - id: insert-license + # name: Add license for all Golang files + # exclude: ^\.github/.*$ + # files: \.go + # args: + #- --comment-style + #- "/*|| */" + #- --license-filepath + #- license-templates/LICENSE.txt + #- --fuzzy-match-generates-todo + # - id: insert-license + # name: Add license for all Java files + # exclude: ^\.github/.*$ + # files: \.java$ + # args: + #- --comment-style + #- "/*|| */" + #- --license-filepath + #- license-templates/LICENSE.txt + #- --fuzzy-match-generates-todo + - id: insert-license Review comment: We are not using bots. I am just using it exactly like Apache Airflow and it's allowed. When run `pre-commit` on the CI the license check would just fail if there was a missing license and the `pre-commit` GitHub Action would fail. When run locally if the license check fails then `pre-commit` would auto insert the missing license. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] jbampton commented on a change in pull request #3624: feat: add pre-commit framework
jbampton commented on a change in pull request #3624: URL: https://github.com/apache/apisix/pull/3624#discussion_r627079184 ## File path: .pre-commit-config.yaml ## @@ -0,0 +1,193 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +--- +default_stages: [commit, push] +default_language_version: + # force all unspecified Python hooks to run python3 + python: python3 +minimum_pre_commit_version: "1.20.0" +repos: + - repo: meta +hooks: + - id: identity + - id: check-hooks-apply + - repo: https://github.com/Lucas-C/pre-commit-hooks +rev: v1.1.10 +hooks: + # - id: insert-license + # name: Add license for all Golang files + # exclude: ^\.github/.*$ + # files: \.go + # args: + #- --comment-style + #- "/*|| */" + #- --license-filepath + #- license-templates/LICENSE.txt + #- --fuzzy-match-generates-todo + # - id: insert-license + # name: Add license for all Java files + # exclude: ^\.github/.*$ + # files: \.java$ + # args: + #- --comment-style + #- "/*|| */" + #- --license-filepath + #- license-templates/LICENSE.txt + #- --fuzzy-match-generates-todo + - id: insert-license +name: Add license for all Lua files +exclude: ^\.github/.*$ +files: \.lua$|\.luacov$ +args: + - --comment-style + - "|--|" + - --license-filepath + - license-templates/LICENSE.txt + - --fuzzy-match-generates-todo + # - id: insert-license + # name: Add license for all Markdown files + # files: \.md$ + # exclude: ^\.github/.*$ + # args: + #- --comment-style + #- "" + #- --license-filepath + #- license-templates/LICENSE.txt + #- --fuzzy-match-generates-todo + - id: insert-license +name: Add license for all Other files Review comment: We are going to group all "Other" file types that are common and have the same type of license. So I just grouped some minor file types to start with. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] jbampton commented on a change in pull request #3624: feat: add pre-commit framework
jbampton commented on a change in pull request #3624: URL: https://github.com/apache/apisix/pull/3624#discussion_r627071814 ## File path: .pre-commit-config.yaml ## @@ -0,0 +1,193 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +--- +default_stages: [commit, push] +default_language_version: + # force all unspecified Python hooks to run python3 + python: python3 +minimum_pre_commit_version: "1.20.0" +repos: + - repo: meta +hooks: + - id: identity + - id: check-hooks-apply + - repo: https://github.com/Lucas-C/pre-commit-hooks +rev: v1.1.10 +hooks: + # - id: insert-license + # name: Add license for all Golang files + # exclude: ^\.github/.*$ + # files: \.go + # args: + #- --comment-style + #- "/*|| */" + #- --license-filepath + #- license-templates/LICENSE.txt + #- --fuzzy-match-generates-todo + # - id: insert-license + # name: Add license for all Java files + # exclude: ^\.github/.*$ + # files: \.java$ + # args: + #- --comment-style + #- "/*|| */" + #- --license-filepath + #- license-templates/LICENSE.txt + #- --fuzzy-match-generates-todo + - id: insert-license +name: Add license for all Lua files +exclude: ^\.github/.*$ +files: \.lua$|\.luacov$ +args: + - --comment-style + - "|--|" + - --license-filepath + - license-templates/LICENSE.txt + - --fuzzy-match-generates-todo + # - id: insert-license + # name: Add license for all Markdown files + # files: \.md$ + # exclude: ^\.github/.*$ + # args: + #- --comment-style + #- "" + #- --license-filepath + #- license-templates/LICENSE.txt + #- --fuzzy-match-generates-todo + - id: insert-license +name: Add license for all Other files +exclude: ^\.github/.*$ +files: > + \.conf$|^Makefile$|\.properties$ +args: + - --comment-style + - "|#|" + - --license-filepath + - license-templates/LICENSE.txt + - --fuzzy-match-generates-todo + - id: insert-license +name: Add license for all Perl files +exclude: ^\.github/.*$ +files: \.t$|\.pm$ +args: + - --comment-style + - "|#|" + - --license-filepath + - license-templates/LICENSE.txt + - --fuzzy-match-generates-todo + - id: insert-license +name: Add license for all Python files +exclude: ^\.github/.*$ +types: [python] +args: + - --comment-style + - "|#|" + - --license-filepath + - license-templates/LICENSE.txt + - --fuzzy-match-generates-todo + - id: insert-license +name: Add license for all Shell files +exclude: ^\.github/.*$ +files: \.sh$ +args: + - --comment-style + - "|#|" + - --license-filepath + - license-templates/LICENSE.txt + - --fuzzy-match-generates-todo + # - id: insert-license + # name: Add license for all XML files + # exclude: ^\.github/.*$ + # files: \.xml$ + # args: + #- --comment-style + #- "" + #- --license-filepath + #- license-templates/LICENSE.txt + #- --fuzzy-match-generates-todo + - id: insert-license +name: Add license for all YAML files +exclude: ^\.github/.*$ +types: [yaml] +files: \.(yaml|yml)$ +args: + - --comment-style + - "|#|" + - --license-filepath + - license-templates/LICENSE.txt + - --fuzzy-match-generates-todo + - repo: https://github.com/pre-commit/pre-commit-hooks +rev: v3.4.0 +hooks: + - id: check-case-conflict + - id: check-json + - id: check-merge-conflict + - id: check-xml + - id: check-yaml + - id: end-of-file-fixer +exclude: ^license-templates/LICENSE\.txt$ + - id: fix-byte-order-marker + - id: mixed-line-ending + - id: trailing-whitespace +
[GitHub] [apisix] jbampton commented on a change in pull request #3624: feat: add pre-commit framework
jbampton commented on a change in pull request #3624: URL: https://github.com/apache/apisix/pull/3624#discussion_r627030926 ## File path: codespell.txt ## @@ -0,0 +1,4 @@ +hel Review comment: `codespell` is a new Python based spell checker that runs on `pre-commit`. This is the `codespell` dictionary and it found some new spelling mistakes. I added docs on this -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] jbampton commented on a change in pull request #3624: feat: add pre-commit framework
jbampton commented on a change in pull request #3624: URL: https://github.com/apache/apisix/pull/3624#discussion_r626372621 ## File path: .pre-commit-config.yaml ## @@ -0,0 +1,72 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +--- +default_stages: [commit, push] +default_language_version: + # force all unspecified Python hooks to run python3 + python: python3 +minimum_pre_commit_version: "1.20.0" +repos: + - repo: meta +hooks: + - id: identity + - id: check-hooks-apply + - repo: https://github.com/pre-commit/pre-commit-hooks +rev: v3.4.0 +hooks: + - id: check-case-conflict + - id: check-json + - id: check-merge-conflict + - id: check-xml + - id: check-yaml + - id: end-of-file-fixer + - id: fix-byte-order-marker + - id: mixed-line-ending + - id: trailing-whitespace +exclude: ^t/node/remote-addr-ipv6.t$ + # - repo: git://github.com/Lucas-C/pre-commit-hooks + # rev: v1.1.9 + # hooks: + #- id: forbid-tabs + #- id: remove-tabs + - repo: https://github.com/codespell-project/codespell +rev: v2.0.0 +hooks: + - id: codespell +name: Check Spelling with codespell +entry: codespell --ignore-words=codespell.txt +exclude: ^docs/ar|^docs/es|^docs/zh|^.git|^.*\.crt$|^.*\.sum$ Review comment: Can you give me a list of files and/or folders I should check or allow ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] jbampton commented on a change in pull request #3624: feat: add pre-commit framework
jbampton commented on a change in pull request #3624: URL: https://github.com/apache/apisix/pull/3624#discussion_r626245528 ## File path: codespell.txt ## @@ -0,0 +1,17 @@ +ba Review comment: Yes the file is necessary but you are correct that we don't need Spanish words in the ignore list. We use all lowercase "words" in the ignore list. I just now ran: ``` codespell --skip="./docs/es,./docs/zh,./.git,./**/*.crt,./**/go.sum" | cut -d' ' -f2 | sort | uniq WithT hel nD nd nin witht ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
