Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use clang-format in tics #2507

Closed
fuzhe1989 opened this issue Jul 28, 2021 · 9 comments · Fixed by #2706
Closed

Use clang-format in tics #2507

fuzhe1989 opened this issue Jul 28, 2021 · 9 comments · Fixed by #2706
Assignees
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@fuzhe1989
Copy link
Contributor

No description provided.

@fuzhe1989 fuzhe1989 self-assigned this Jul 28, 2021
@jiaqizho
Copy link
Contributor

can we add clang-format checking in git-hook?

@fuzhe1989
Copy link
Contributor Author

@jiaqizho I were told that we already have a clang-format: https://github.com/pingcap/tics/blob/master/format.sh

What we need is a per-commit test set which will run after ci to ensure not slow down it. @solotzg do you have any plan about this?

@jiaqizho
Copy link
Contributor

@jiaqizho I were told that we already have a clang-format: https://github.com/pingcap/tics/blob/master/format.sh

What we need is a per-commit test set which will run after ci to ensure not slow down it. @solotzg do you have any plan about this?

I known.

I mean add format.sh in git hook pre-commit. https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks

so before commit , it can check format and remind developers which files are not formatted.

@fuzhe1989
Copy link
Contributor Author

@jiaqizho I'm not sure if it should block the merge. Anyway a pre-check is good, or at least we can manually use it to format in batch.

@solotzg
Copy link
Contributor

solotzg commented Jul 29, 2021

@JaySon-Huang
Copy link
Contributor

Maybe we should move the format-tics.py to the tics repo (and add a pre-check)?

@solotzg
Copy link
Contributor

solotzg commented Jul 30, 2021

Maybe we can modify format-tics.py to check whether commits in PR are formated before running ci-build.

@solotzg
Copy link
Contributor

solotzg commented Jul 30, 2021

Maybe we should move the format-tics.py to the tics repo (and add a pre-check)?

However, there are at least 3 kinds of .clang-format in tics repo. Shall we unify standards?

@fuzhe1989
Copy link
Contributor Author

@solotzg let's do it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants