search

tempesta-tech / tempesta

All-in-one solution for high performance web content delivery and advanced protection against DDoS and web attacks
https://tempesta-tech.com/
GNU General Public License v2.0
607 stars 104 forks source link

Integrate `checkpatch.pl` to our build/commit pipeline #2142

Open krizhanovsky opened 1 month ago

krizhanovsky commented 1 month ago

Motivation

It's too time consuming to comment coding style violations in each pull request.

Scope

The Linux kernel provides checkpatch.pl script, which verifies patches along regular C files. We should either integrate it into git commit hook or github actions to be run on each pull request, that an author can fix all the violation w/o reviewer.

An example:

$ ./scripts/checkpatch.pl --no-tree -f ../tempesta/fw/sync_socket.h 
WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#1: FILE: ../tempesta/fw/sync_socket.h:1:
+/**

WARNING: do not add new typedefs
#32: FILE: ../tempesta/fw/sync_socket.h:32:
+typedef struct ss_proto_t {

WARNING: do not add new typedefs
#66: FILE: ../tempesta/fw/sync_socket.h:66:
+typedef struct tfw_conn_t TfwConn;

WARNING: do not add new typedefs
#69: FILE: ../tempesta/fw/sync_socket.h:69:
+typedef struct ss_hooks {

WARNING: do not add new typedefs
#95: FILE: ../tempesta/fw/sync_socket.h:95:
+typedef struct {

total: 0 errors, 5 warnings, 188 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

../tempesta/fw/sync_socket.h has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

Adjustments

We still have small changes from the kernel Coding style, which must be changed in the script:

kingluo commented 4 weeks ago

When we start developing, we need to enable git hooks: https://github.com/tempesta-tech/tempesta/wiki/Development-guidelines#coding-style

There are many existing errors and warnings that we need developers to fix. It is not necessary to do the fixes for all source files at once, the pre-commit hook will only check the modified/added/copied files for the current commit, so it is best to do it gradually.

Most can be fixed automatically with the --fix-inplace option, others can only be done manually.

krizhanovsky commented 4 weeks ago

@kingluo yeah, I didn't mean to fix coding style around the whole project within this issue.

For now I just want to make all the code authors to validate their own code (i.e. we need to run the tool of patch/diffs, not the .c/.h files) and fix their code and the code around the new code.

This task should get rid of time spending on reviewing coding style issues in PRs.