[GitHub] [apisix] jbampton commented on a change in pull request #3624: feat: add pre-commit framework

2021-05-05 Thread GitBox


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

2021-05-05 Thread GitBox


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

2021-05-05 Thread GitBox


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

2021-05-05 Thread GitBox


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

2021-05-05 Thread GitBox


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

2021-05-05 Thread GitBox


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

2021-05-05 Thread GitBox


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

2021-05-04 Thread GitBox


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