Re: Add `pre-commit-hooks` for pre-commit checks
ocal > > git config, not forcefully restricted by the git-hook script. For > some > > developer who has followed the ContributeGuide [1] to config his IDE > well, > > the formatter will be automatically run after file saving. This tool > is > > less helpful for them. > > > > [1] > > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fiotdb.apache.org%2FDevelopment%2FContributeGuide.html%23code-formattingdata=05%7C01%7C%7C8628dc8797cb4c2a08da54238abb%7C84df9e7fe9f640afb435%7C1%7C0%7C637914808878106412%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=widk89VLhU%2B41G%2BaWV8I2nKHzJttjecF3P1%2B7kWxqRQ%3Dreserved=0 > > > > -邮件原件- > > 发件人: 林地宁宁 > > 发送时间: 2022年6月20日 19:21 > > 收件人: dev@iotdb.apache.org > > 主题: Add `pre-commit-hooks` for pre-commit checks > > > > Hi guys! > > > > AFAIK, we have some style checks in CI/CD. As a novice here, I find > it > > quite annoying to remember the checks before I commit. > > > > So how about adding some pre-commit hooks to do these checks for us, > such > > as `mvn spotless:apply`. > > > > Though there are some small questions here: > > 1. Which checks or operations should be done by pre-commit hooks? > > 2. If a pre-commit check failed, should it throw the error and stop > the > > commit? Or just apply the modifications to finish the commit? > > > > Any suggestions? > > > > > > -- > > Kind regards, > > Ke Lin > > > >
Re: Add `pre-commit-hooks` for pre-commit checks
LOL I make it! PR https://github.com/apache/iotdb/pull/6380 gives the details. I wonder if I have to place the doc to somewhere else? Eric Pai 于2022年6月21日周二 09:50写道： > Got it! If we use shell script, we should consider both mac/Linux/Windows > environment, as we have lots of developers using Windows, coding with > powershell is not easy. and the usage of same command may be different in > Mac and Linux. > If we use Python or Ruby, or any other interpreted programming language, > we should check whether the developing machine has those interpreters > before executing. > So it's better to add the pre-hook and disable it by default. Developers > can set some environment variables as you said to enable it if needed. > > 在 2022/6/21 00:40，“Ke Lin” 写入: > > Thanks for your advice! > > After reading the doc of git-hooks > < > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-scm.com%2Fbook%2Fen%2Fv2%2FCustomizing-Git-Git-Hooksdata=05%7C01%7C%7Ceee6c271a0fc45cfb75c08da52db9b67%7C84df9e7fe9f640afb435%7C1%7C0%7C637913400436327114%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=meBJtA0T0h99WtWX9dtDDTY2HtDsTeEU6qGUmmtzYCk%3Dreserved=0>, > I probably > found some solutions: > 1. Git-hooks are executable scripts in fact, e.g., shell scripts, ruby > or > python. So we can define our rules for the linter reports. > 2. To reduce the maven overhead, we may do some tricks to the > executable > scripts and validate only on the changed code. But I'm not pretty sure > if > it's viable. > 3. Env variables should work well with the hook scripts to control the > pre-commit pipeline. > > Eric Pai 于2022年6月20日周一 20:55写道： > > > This is a very helpful tool for developers! > > > > In IoTDB we have not only code formatter, the spotless, but also the > code > > linter, checkstyle. In maven we can run ‘mvn validate‘ to run checks > of > > both the linter and formatter, or 'mvn validate -pl ' > to check > > one or more particular modules. However, unlike 'mvn spotless:apply', > > there's no way to automatically fix the linter error, as it needs > more > > semantic information. E.g. we forbid to use wildcard import in Java > codes, > > but the linter doesn’t know which class we really want to import, so > I have > > some suggestions for this tool. > > > > 1. If we want to let pre-commit hook reformat the code automatically, > > could we also report whether there're any violations of the linter > rules? > > 2. Running a maven command is a heavy operation as it will launch a > JVM. I > > have tested in my desktop running 'mvn validate' and 'mvn > spotless:apply'. > > They cost 90 and 55 seconds, it's too long to wait for a commit. Can > the > > hook be wisely to choose the modules whose codes are updated to run > only? > > For example I just change the codes in confignode module, so it's not > > necessary to run the check in the module server, jdbc, session... > > 3. It's necessary to give some docs to explain how to enable/disable > this > > tool easily. It means that developers can change this through their > local > > git config, not forcefully restricted by the git-hook script. For > some > > developer who has followed the ContributeGuide [1] to config his IDE > well, > > the formatter will be automatically run after file saving. This tool > is > > less helpful for them. > > > > [1] > > > https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fiotdb.apache.org%2FDevelopment%2FContributeGuide.html%23code-formattingdata=05%7C01%7C%7Ceee6c271a0fc45cfb75c08da52db9b67%7C84df9e7fe9f640afb435%7C1%7C0%7C637913400436327114%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=pysbHzDTYW1dW4wASKkMFORKJNJX8KI9qqJif63FtfQ%3Dreserved=0 > > > > -邮件原件- > > 发件人: 林地宁宁 > > 发送时间: 2022年6月20日 19:21 > > 收件人: dev@iotdb.apache.org > > 主题: Add `pre-commit-hooks` for pre-commit checks > > > > Hi guys! > > > > AFAIK, we have some style checks in CI/CD. As a novice here, I find > it > > quite annoying to remember the checks before I commit. > > > > So how about adding some pre-commit hooks to do these checks for us, > such > > as `mvn spotless:apply`. > > > > Though there are some small questions here: > > 1. Which checks or operations should be done by pre-commit hooks? > > 2. If a pre-commit check failed, should it throw the error and stop > the > > commit? Or just apply the modifications to finish the commit? > > > > Any suggestions? > > > > > > -- > > Kind regards, > > Ke Lin > > > >
Re: Add `pre-commit-hooks` for pre-commit checks
Got it! If we use shell script, we should consider both mac/Linux/Windows
environment, as we have lots of developers using Windows, coding with
powershell is not easy. and the usage of same command may be different in Mac
and Linux.
If we use Python or Ruby, or any other interpreted programming language, we
should check whether the developing machine has those interpreters before
executing.
So it's better to add the pre-hook and disable it by default. Developers can
set some environment variables as you said to enable it if needed.
在 2022/6/21 00:40，“Ke Lin” 写入:
Thanks for your advice!
After reading the doc of git-hooks
<https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-scm.com%2Fbook%2Fen%2Fv2%2FCustomizing-Git-Git-Hooksdata=05%7C01%7C%7Ceee6c271a0fc45cfb75c08da52db9b67%7C84df9e7fe9f640afb435%7C1%7C0%7C637913400436327114%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=meBJtA0T0h99WtWX9dtDDTY2HtDsTeEU6qGUmmtzYCk%3Dreserved=0>,
I probably
found some solutions:
1. Git-hooks are executable scripts in fact, e.g., shell scripts, ruby or
python. So we can define our rules for the linter reports.
2. To reduce the maven overhead, we may do some tricks to the executable
scripts and validate only on the changed code. But I'm not pretty sure if
it's viable.
3. Env variables should work well with the hook scripts to control the
pre-commit pipeline.
Eric Pai 于2022年6月20日周一 20:55写道：
> This is a very helpful tool for developers!
>
> In IoTDB we have not only code formatter, the spotless, but also the code
> linter, checkstyle. In maven we can run ‘mvn validate‘ to run checks of
> both the linter and formatter, or 'mvn validate -pl ' to
check
> one or more particular modules. However, unlike 'mvn spotless:apply',
> there's no way to automatically fix the linter error, as it needs more
> semantic information. E.g. we forbid to use wildcard import in Java codes,
> but the linter doesn’t know which class we really want to import, so I
have
> some suggestions for this tool.
>
> 1. If we want to let pre-commit hook reformat the code automatically,
> could we also report whether there're any violations of the linter rules?
> 2. Running a maven command is a heavy operation as it will launch a JVM. I
> have tested in my desktop running 'mvn validate' and 'mvn spotless:apply'.
> They cost 90 and 55 seconds, it's too long to wait for a commit. Can the
> hook be wisely to choose the modules whose codes are updated to run only?
> For example I just change the codes in confignode module, so it's not
> necessary to run the check in the module server, jdbc, session...
> 3. It's necessary to give some docs to explain how to enable/disable this
> tool easily. It means that developers can change this through their local
> git config, not forcefully restricted by the git-hook script. For some
> developer who has followed the ContributeGuide [1] to config his IDE well,
> the formatter will be automatically run after file saving. This tool is
> less helpful for them.
>
> [1]
>
https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fiotdb.apache.org%2FDevelopment%2FContributeGuide.html%23code-formattingdata=05%7C01%7C%7Ceee6c271a0fc45cfb75c08da52db9b67%7C84df9e7fe9f640afb435%7C1%7C0%7C637913400436327114%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=pysbHzDTYW1dW4wASKkMFORKJNJX8KI9qqJif63FtfQ%3Dreserved=0
>
> -邮件原件-----
> 发件人: 林地宁宁
> 发送时间: 2022年6月20日 19:21
> 收件人: dev@iotdb.apache.org
> 主题: Add `pre-commit-hooks` for pre-commit checks
>
> Hi guys!
>
> AFAIK, we have some style checks in CI/CD. As a novice here, I find it
> quite annoying to remember the checks before I commit.
>
> So how about adding some pre-commit hooks to do these checks for us, such
> as `mvn spotless:apply`.
>
> Though there are some small questions here:
> 1. Which checks or operations should be done by pre-commit hooks?
> 2. If a pre-commit check failed, should it throw the error and stop the
> commit? Or just apply the modifications to finish the commit?
>
> Any suggestions?
>
>
> --
> Kind regards,
> Ke Lin
>
Re: Add `pre-commit-hooks` for pre-commit checks
Thanks for your advice! After reading the doc of git-hooks <https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks>, I probably found some solutions: 1. Git-hooks are executable scripts in fact, e.g., shell scripts, ruby or python. So we can define our rules for the linter reports. 2. To reduce the maven overhead, we may do some tricks to the executable scripts and validate only on the changed code. But I'm not pretty sure if it's viable. 3. Env variables should work well with the hook scripts to control the pre-commit pipeline. Eric Pai 于2022年6月20日周一 20:55写道： > This is a very helpful tool for developers! > > In IoTDB we have not only code formatter, the spotless, but also the code > linter, checkstyle. In maven we can run ‘mvn validate‘ to run checks of > both the linter and formatter, or 'mvn validate -pl ' to check > one or more particular modules. However, unlike 'mvn spotless:apply', > there's no way to automatically fix the linter error, as it needs more > semantic information. E.g. we forbid to use wildcard import in Java codes, > but the linter doesn’t know which class we really want to import, so I have > some suggestions for this tool. > > 1. If we want to let pre-commit hook reformat the code automatically, > could we also report whether there're any violations of the linter rules? > 2. Running a maven command is a heavy operation as it will launch a JVM. I > have tested in my desktop running 'mvn validate' and 'mvn spotless:apply'. > They cost 90 and 55 seconds, it's too long to wait for a commit. Can the > hook be wisely to choose the modules whose codes are updated to run only? > For example I just change the codes in confignode module, so it's not > necessary to run the check in the module server, jdbc, session... > 3. It's necessary to give some docs to explain how to enable/disable this > tool easily. It means that developers can change this through their local > git config, not forcefully restricted by the git-hook script. For some > developer who has followed the ContributeGuide [1] to config his IDE well, > the formatter will be automatically run after file saving. This tool is > less helpful for them. > > [1] > https://iotdb.apache.org/Development/ContributeGuide.html#code-formatting > > -邮件原件----- > 发件人: 林地宁宁 > 发送时间: 2022年6月20日 19:21 > 收件人: dev@iotdb.apache.org > 主题: Add `pre-commit-hooks` for pre-commit checks > > Hi guys! > > AFAIK, we have some style checks in CI/CD. As a novice here, I find it > quite annoying to remember the checks before I commit. > > So how about adding some pre-commit hooks to do these checks for us, such > as `mvn spotless:apply`. > > Though there are some small questions here: > 1. Which checks or operations should be done by pre-commit hooks? > 2. If a pre-commit check failed, should it throw the error and stop the > commit? Or just apply the modifications to finish the commit? > > Any suggestions? > > > -- > Kind regards, > Ke Lin >
回复: Add `pre-commit-hooks` for pre-commit checks
This is a very helpful tool for developers! In IoTDB we have not only code formatter, the spotless, but also the code linter, checkstyle. In maven we can run ‘mvn validate‘ to run checks of both the linter and formatter, or 'mvn validate -pl ' to check one or more particular modules. However, unlike 'mvn spotless:apply', there's no way to automatically fix the linter error, as it needs more semantic information. E.g. we forbid to use wildcard import in Java codes, but the linter doesn’t know which class we really want to import, so I have some suggestions for this tool. 1. If we want to let pre-commit hook reformat the code automatically, could we also report whether there're any violations of the linter rules? 2. Running a maven command is a heavy operation as it will launch a JVM. I have tested in my desktop running 'mvn validate' and 'mvn spotless:apply'. They cost 90 and 55 seconds, it's too long to wait for a commit. Can the hook be wisely to choose the modules whose codes are updated to run only? For example I just change the codes in confignode module, so it's not necessary to run the check in the module server, jdbc, session... 3. It's necessary to give some docs to explain how to enable/disable this tool easily. It means that developers can change this through their local git config, not forcefully restricted by the git-hook script. For some developer who has followed the ContributeGuide [1] to config his IDE well, the formatter will be automatically run after file saving. This tool is less helpful for them. [1] https://iotdb.apache.org/Development/ContributeGuide.html#code-formatting -邮件原件- 发件人: 林地宁宁 发送时间: 2022年6月20日 19:21 收件人: dev@iotdb.apache.org 主题: Add `pre-commit-hooks` for pre-commit checks Hi guys! AFAIK, we have some style checks in CI/CD. As a novice here, I find it quite annoying to remember the checks before I commit. So how about adding some pre-commit hooks to do these checks for us, such as `mvn spotless:apply`. Though there are some small questions here: 1. Which checks or operations should be done by pre-commit hooks? 2. If a pre-commit check failed, should it throw the error and stop the commit? Or just apply the modifications to finish the commit? Any suggestions? -- Kind regards, Ke Lin
Add `pre-commit-hooks` for pre-commit checks
Hi guys! AFAIK, we have some style checks in CI/CD. As a novice here, I find it quite annoying to remember the checks before I commit. So how about adding some pre-commit hooks to do these checks for us, such as `mvn spotless:apply`. Though there are some small questions here: 1. Which checks or operations should be done by pre-commit hooks? 2. If a pre-commit check failed, should it throw the error and stop the commit? Or just apply the modifications to finish the commit? Any suggestions? -- Kind regards, Ke Lin
