[MERGED] Pre-commit hooks
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
Re: [MERGED] Pre-commit hooks
Translation merges are huge, they can touch many files.
- adrianinsaval
- Veteran
- Posts: 5551
- Joined: Thu Apr 05, 2018 5:15 pm
Re: [MERGED] Pre-commit hooks
what's the limiting factor that caps this at 300 files? I don't understand why a computer wouldn't be able to handle more
Re: [MERGED] Pre-commit hooks
GH APIadrianinsaval wrote: ↑Sun May 28, 2023 8:23 pm what's the limiting factor that caps this at 300 files? I don't understand why a computer wouldn't be able to handle more
- adrianinsaval
- Veteran
- Posts: 5551
- Joined: Thu Apr 05, 2018 5:15 pm
Re: [MERGED] Pre-commit hooks
Is it hard to implement logic to go through the following pages if there are 300 files in the response? In any case we can get it working with the 300 file limit and later see how we can go through the multiple pages if we need it
Re: [MERGED] Pre-commit hooks
There is no pagination. This is a hard limit.
- adrianinsaval
- Veteran
- Posts: 5551
- Joined: Thu Apr 05, 2018 5:15 pm
Re: [MERGED] Pre-commit hooks
The compare API between 2 SHA.
https://docs.github.com/en/repositories ... iff-limitswhat I read said up to 300 changes per page but up to 3000 total.
Sure we can but today in Prepare step we haven't a clone of the repo, what I tried to avoid to add.Also, we can't we just use a git command to get the changed files?
Well, actually I succeeded getting the file diff with a very lightweight mechanism.
But then running the pre-commit on very large set of files (I tested with 4000+ file in the diff) takes literally ages.
I think our best option is to get a clone of the repo to run it.
Now should we run in Prepare ?
* Pros : will be a very strict reject if fails
* Cons : is a bottleneck as all subsequent workflows are waiting for it to finish
Or add in Lint ?
Or in another workflow ? (but I don't think it really makes sense)
I'll do some more tests to get idea of running time in each situation.
- adrianinsaval
- Veteran
- Posts: 5551
- Joined: Thu Apr 05, 2018 5:15 pm
Re: [MERGED] Pre-commit hooks
I expect Lint does have a clone of the repo?
Re: [MERGED] Pre-commit hooks
Yes it already has one because Clang-tidy needs it.
- adrianinsaval
- Veteran
- Posts: 5551
- Joined: Thu Apr 05, 2018 5:15 pm
Re: [MERGED] Pre-commit hooks
I think that applies to the web interface more than the API, see: https://docs.github.com/en/rest/pulls/p ... ests-files and https://docs.github.com/en/rest/commits ... t-a-commit but if the commit hook takes an absurd amount of time to run on too many files 300 may be a reasonable limit