Document the clang-format flow in CONTRIBUTING.md #659
Conversation
Signed-off-by: Anuragkillswitch <[email protected]>
0cd3972
to
1a9dff4
Compare
|
@PingXie Can you please take a look ^_^ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #659 +/- ##
============================================
+ Coverage 70.17% 70.19% +0.01%
============================================
Files 110 110
Lines 60077 60078 +1
============================================
+ Hits 42160 42171 +11
+ Misses 17917 17907 -10 |
| [Clang Format Check/clang-format-check] 🏁 Job succeeded | ||
| ``` | ||
| ``` | ||
|
|
There was a problem hiding this comment.
We should probably keep the DCO at the top as this is the most important information IMO for any new contributor. Also while you are at it, can you add some git commands to show how to add the DCO? I noticed that the commands were not obvious to some folks in the past.
Co-authored-by: Binbin <[email protected]>
Co-authored-by: Ping Xie <[email protected]>
| @@ -19,6 +19,74 @@ The Valkey project is led by a Technical Steering Committee, whose responsibilit | |||
| * Want to help with documentation? [Move on to valkey-doc](https://github.com/valkey-io/valkey-doc) | |||
| * Report a vulnerability? See [SECURITY.md](SECURITY.md) | |||
|
|
|||
| ## How to Install clang-format 18 on a Debian Box | |||
There was a problem hiding this comment.
I don't really want to maintain an installation guide of clang-format for debian in this repository. Is there some better place we can point to? Do we need at least clang-format version 18 for this to run correctly?
There was a problem hiding this comment.
yes 18 is the most compatible one. I think we can go higher but the installation becomes challenging.
What would you recommend for this documentation? Would it work we clarify this being debian specific (so we can extend it in the future to other platforms)?
There was a problem hiding this comment.
I install clang-format version 14 on Ubuntu 22, it works fine. But I have no chance to older version, So we can say:
First run git clang-format, If you still meet an error, please upgrade your clang-format to latest version.
|
|
||
| ## How to Run clang-format-check Action Locally | ||
|
|
||
| 1. Get [`act`](https://github.com/nektos/act). |
There was a problem hiding this comment.
| 1. Get [`act`](https://github.com/nektos/act). | |
| 1. Install [`act`](https://nektosact.com/installation/index.html). |
| 2. Commit the changes locally: | ||
| ```sh | ||
| git commit -a -s | ||
| ``` |
There was a problem hiding this comment.
| 2. Commit the changes locally: | |
| ```sh | |
| git commit -a -s | |
| ``` |
Instead, I think you should indicate it runs on what is current committed locally.
| @@ -19,6 +19,74 @@ The Valkey project is led by a Technical Steering Committee, whose responsibilit | |||
| * Want to help with documentation? [Move on to valkey-doc](https://github.com/valkey-io/valkey-doc) | |||
| * Report a vulnerability? See [SECURITY.md](SECURITY.md) | |||
|
|
|||
| ## How to Install clang-format 18 on a Debian Box | |||
There was a problem hiding this comment.
It would also be good to put this under a code format section, instead of a section for installing clang format. It would be nice to include an example for how to run it.
| sudo act -j clang-format-check | ||
| ``` | ||
|
|
||
| ### Here is a Failure Output |
There was a problem hiding this comment.
I think these two sections are overkill. The most useful part is the git diff part, so maybe just keep that? If there is no diff, then it will pass.
|
I was wondering if we can do something like the following for the installation steps @PingXie @madolson @enjoy-binbin @hwware |
|
The 30 second demo thing look cool, but i don't like this kind of fancy stuff on projects |
Solves #649.