[
https://issues.apache.org/jira/browse/BEAM-9274?focusedWorklogId=385424&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-385424
]
ASF GitHub Bot logged work on BEAM-9274:
----------------------------------------
Author: ASF GitHub Bot
Created on: 11/Feb/20 20:15
Start Date: 11/Feb/20 20:15
Worklog Time Spent: 10m
Work Description: chadrik commented on pull request #10810: [BEAM-9274]
Support running yapf in a git pre-commit hook
URL: https://github.com/apache/beam/pull/10810#discussion_r377873324
##########
File path: .pre-commit-config.yaml
##########
@@ -0,0 +1,32 @@
+# Licensed 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.
+
+repos:
+ - repo: https://github.com/pre-commit/mirrors-yapf
+ # this rev is a release tag in the repo above and corresponds with a yapf
+ # version. make sure this matches the version of yapf in tox.ini.
+ rev: v0.29.0
+ hooks:
+ - id: yapf
+ files: ^sdks/python/apache_beam/
+ # keep these in sync with sdks/python/.yapfignore
Review comment:
First, some background:
pre-commit triggers its hooks based on changed files, matching based on 3
values:
- file type (e.g. it has a mapping from "python" to , ".py", etc)
- files: include patttern
- exclude: exclude pattern
pre-commit always passes an explicit list of changed files to the underlying
tool, and never relies on recursive flags.
Ok, on to your question. Our exclude patterns cover autogenerated files.
So, if a new autogenerated file were added to the repo which was excluded in
.yapfignore but not in .pre-commit-config.yaml (which I think would be the most
common "failure" scenario), then the first time that a developer with
pre-commit enabled changed that file (say, by regenerating it) and tried to
commit those changes, yapf would autoformat it, and then pre-commit would fail
the commit with a message like this:
```
yapf.....................................................................Failed
- hook id: yapf
- files were modified by this hook
```
At this point hopefully the developer realizes that the failure is due to
yapf, and finds the trail of comments leading them to .pre-commit-config.yaml.
Worst case scenario is that they commit it with the autoformatted changes,
which would be pretty innocuous, and would not fail any jenkins tests because
those same autogenerated files are excluded from pylint.
Another thing to note is that this is all just a convenience for developers.
Jenkins remains our last line of defense.
Also note that it's possible to invoke your lint tools using pre-commit
within tox, so that you can consolidate all of your includes and excludes
across all tools into one file, the pre-commit-config.yaml. This is what we do
where I work.
----------------------------------------------------------------
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
Issue Time Tracking
-------------------
Worklog Id: (was: 385424)
Time Spent: 1h (was: 50m)
> Support running yapf in a git pre-commit hook
> ---------------------------------------------
>
> Key: BEAM-9274
> URL: https://issues.apache.org/jira/browse/BEAM-9274
> Project: Beam
> Issue Type: Improvement
> Components: sdk-py-core
> Reporter: Chad Dombrova
> Assignee: Chad Dombrova
> Priority: Major
> Time Spent: 1h
> Remaining Estimate: 0h
>
> As a developer I want to be able to automatically run yapf before I make a
> commit so that I don't waste time with failures on jenkins.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
