You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/01/09 00:52:56 UTC
[GitHub] [druid] maytasm3 opened a new pull request #9157: Add git
pre-commit hook to source control
maytasm3 opened a new pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157
### Description
- Added checkstyle pre-commit hook. (https://github.com/hdpe/maven-checkstyle-pre-commit/blob/master/pre-commit). Can verify checkstyle on changed modules before committing saving cycle from avoiding the checkstyle failing later.
- Made the above hook source control and can be setup easily by running the setup-hooks.sh (you don't have to download them manually and do chmod etc.)
This PR has:
- [x] been self-reviewed.
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/licenses.yaml)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9157: Add
git pre-commit hook to source control
Posted by GitBox <gi...@apache.org>.
maytasm3 commented on a change in pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#discussion_r369320817
##########
File path: hooks/pre-push.sh
##########
@@ -0,0 +1,17 @@
+#!/bin/bash -eu
+# 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.
+
+mvn checkstyle:checkstyle --fail-at-end
Review comment:
I saw the script at https://github.com/hdpe/maven-checkstyle-pre-commit/blob/master/pre-commit but the licensing is not clear so decided not to add it. It takes about 30s to runs checkstyle on everything and this is only done pre push (so should be less often then if we do pre commit). I think 30s is not too bad considering that the CI thing takes 1-2 hours and we can save resource on CI
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 closed pull request #9157: Add git pre-commit hook
to source control
Posted by GitBox <gi...@apache.org>.
maytasm3 closed pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9157: Add
git pre-commit hook to source control
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#discussion_r369319549
##########
File path: setup-hooks.sh
##########
@@ -0,0 +1,17 @@
+#!/bin/bash -eu
Review comment:
Do you want to add some documentation to [druid/dev](https://github.com/apache/druid/tree/master/dev)?
We may want to give folks the option of where to install the checkstyle (i.e., pre-push or pre-commit).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jon-wei commented on issue #9157: Add git pre-commit hook
to source control
Posted by GitBox <gi...@apache.org>.
jon-wei commented on issue #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#issuecomment-572330971
Hm, the licensing on https://github.com/hdpe/maven-checkstyle-pre-commit/blob/master/pre-commit is unclear, I don't think we should include anything from there
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 commented on issue #9157: Add git pre-commit hook
to source control
Posted by GitBox <gi...@apache.org>.
maytasm3 commented on issue #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#issuecomment-572337873
Changed hook to pre-push and simply hook to run all checkstyle
The checkstyle takes ~6 secs.
This is also optional too. If you don’t run the setup-hook script then the hook wont be applied.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9157: Add
git pre-commit hook to source control
Posted by GitBox <gi...@apache.org>.
maytasm3 commented on a change in pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#discussion_r369325020
##########
File path: setup-hooks.sh
##########
@@ -0,0 +1,17 @@
+#!/bin/bash -eu
Review comment:
Added to intellij-setup.md
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 commented on issue #9157: Add git pre-commit hook
to source control
Posted by GitBox <gi...@apache.org>.
maytasm3 commented on issue #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#issuecomment-576946469
Added Apache license header.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9157: Add
git pre-commit hook to source control
Posted by GitBox <gi...@apache.org>.
maytasm3 commented on a change in pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#discussion_r369322328
##########
File path: setup-hooks.sh
##########
@@ -0,0 +1,17 @@
+#!/bin/bash -eu
Review comment:
Actually I don't know where's a good place to add that
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9157: Add
git pre-commit hook to source control
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#discussion_r369319128
##########
File path: hooks/pre-push.sh
##########
@@ -0,0 +1,17 @@
+#!/bin/bash -eu
+# 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.
+
+mvn checkstyle:checkstyle --fail-at-end
Review comment:
If this runs checkstyle on everything (versus just the modified files) it'll probably take 10s of seconds to run
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9157: Add
git pre-commit hook to source control
Posted by GitBox <gi...@apache.org>.
maytasm3 commented on a change in pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#discussion_r369320924
##########
File path: hooks/pre-push.sh
##########
@@ -0,0 +1,17 @@
+#!/bin/bash -eu
+# 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.
+
+mvn checkstyle:checkstyle --fail-at-end
Review comment:
Also the hook is optional. If you don't run the setup script then it wont be applied.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9157: Add
git pre-commit hook to source control
Posted by GitBox <gi...@apache.org>.
maytasm3 commented on a change in pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#discussion_r369321664
##########
File path: setup-hooks.sh
##########
@@ -0,0 +1,17 @@
+#!/bin/bash -eu
Review comment:
Added instruction to teamcity.md
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jon-wei commented on a change in pull request #9157: Add
git pre-commit hook to source control
Posted by GitBox <gi...@apache.org>.
jon-wei commented on a change in pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#discussion_r370386957
##########
File path: setup-hooks.sh
##########
@@ -0,0 +1,17 @@
+#!/bin/bash -eu
Review comment:
@maytasm3 did the intellij-setup.md change get pushed?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jon-wei commented on issue #9157: Add git pre-commit hook
to source control
Posted by GitBox <gi...@apache.org>.
jon-wei commented on issue #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#issuecomment-572741785
@maytasm3 The new files will need the Apache license header:
```
# 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.
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 commented on a change in pull request #9157: Add
git pre-commit hook to source control
Posted by GitBox <gi...@apache.org>.
maytasm3 commented on a change in pull request #9157: Add git pre-commit hook to source control
URL: https://github.com/apache/druid/pull/9157#discussion_r395975723
##########
File path: setup-hooks.sh
##########
@@ -0,0 +1,17 @@
+#!/bin/bash -eu
Review comment:
@jon-wei Lost my branch. Opened a new PR with the intellij-setup.md change.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org