Comments (7)
Pre commits only work, if all people who make a PR actually use them, right? I am not against this, but I would argue that we have to have correct files even if the PR is from someone who does not check with a pre-commit hook before they send it in.
No, you don't have to use it as a Git hook. That's how it started out and hence the name. But I personally don't use it like that in any project. Instead, I trigger it manually by running pre-commit run
. That then runs whatever is configured. To replicate the current configuration of this project, we would only have clang-format
. But clang-format-docs
could be easily added and would then run as well. pre-commit run
returns a non-zero exit code in case it made any changes. That is what would be used in CI, so that all configured checks are also part of a CI run. So no matter what people use locally to create PRs, we would have all checks in place. The big upside is that pre-commit is a widely used project, so the probability of things not really working (as has happened now with the custom script) is really low.
You know what, let me put together a PR today or tomorrow to show you how it would look like.
from cpp.
A fix for the issue is here: #835
from cpp.
However, there's another issue in the check of all files never happens. It is supposed to be run for each commit on main
. However, since the if
in the workflow checks for a branch named master
, this never becomes true (it's called main
). Fixing that is trivial of course, but we should probably also re-format all files in the same PR.
I can certainly do that. Just let me know. There will be a few hundred files that need reformatting.
from cpp.
I can certainly do that. Just let me know. There will be a few hundred files that need reformatting.
Please go forward, this is very good for the code base.
Do you know if we could easily check the correct formatting for the code blocks in the .md files?
from cpp.
I can certainly do that. Just let me know. There will be a few hundred files that need reformatting.
Please go forward, this is very good for the code base.
Alright, I can do that. However, that should come after the check is in place. So could you have a look at #835 please? That fixes the script and will make sure that future PRs are properly formatted.
Fixing the check that runs for each commit on main
and reformatting existing files I would do in a follow-up PR.
Do you know if we could easily check the correct formatting for the code blocks in the .md files?
I haven't done this yet, but clang-format-docs looks promising. I have just run it on a random concepts/headers/introduction.md
and it looks like it's working properly. Running the formatting on .md
files and putting a CI check in place I would also do in a follow-up PR then.
Instead of extending the existing script or adding another custom script to do this, would you be open to use pre-commit for this? I have had quite positive experiences with this in other projects. Although it was meant to be run as a Git hook originally, you don't have to use it like that. The current workflow can stay in place, but instead of a custom script, you would run pre-commit run
locally to run the reformatting tool(s).
from cpp.
Pre commits only work, if all people who make a PR actually use them, right? I am not against this, but I would argue that we have to have correct files even if the PR is from someone who does not check with a pre-commit hook before they send it in.
from cpp.
Alright, so I put it up for my fork and it seems to be working fine. The commit messages would need some cleanup, but everything is there to get the idea.
The PR is here. pre-commit is configured via .pre-commit-config.yaml. In addition to the clang-format check, there are a few standard checks. Those are not mandatory, but don't hurt either, so I kept them in. So this config plus the pre-commit
command is all you need. Install pre-commit locally and you're good to go. My typical workflow is running pre-commit run --all
locally. Without the --all
it only looks at the current commit, so that's what would be used in a Git hook (via pre-commit install
you can locally create the hook, but that's optional).
For the CI setup I created an extra workflow. That just uses the pre-commit action. I noticed in the meantime that it always runs with --all
, so that special handling of PR vs main
is not necessary and should be removed. If you want to see a CI run where the check fails, here is an example.
from cpp.
Related Issues (20)
- Missing semi-colon in 'interest-is-interesting' tutorial HOT 4
- Building a training set of tags for cpp HOT 26
- fix: spelling mistake in classes docs
- pop-count: The header file tab is missing. HOT 3
- doctor-data: typo HOT 2
- Doctor Data skeleton contains garbage HOT 2
- ESP-AT: CMake error: Does not match the generator used previously: Ninja HOT 3
- feat: add approaches for leap exercise
- Make double testing more robust for freelancer rates cocnept exercise
- Update test cases HOT 37
- Add #48in24 featured exercises HOT 19
- Tweak troll exercise
- Passthrough mode - I receive ERROR when I send a packet HOT 2
- log-levels concept exercise needs a more fleshed-out stub HOT 3
- parallell-letter-frequency tests are missing some assertions HOT 2
- CI: Only run tests on exercises, when exercise relevant files are changed
- disregard, wrong project :-/ HOT 1
- doctor-data task 4 "get_older_bob" is confusing HOT 1
- robot-name: tests are not sufficient when using mt19937 HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from cpp.