[MERGED] Pre-commit hooks

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
User avatar
chennes
Veteran
Posts: 3877
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: [MERGED] Pre-commit hooks

Post by chennes »

Translation merges are huge, they can touch many files.
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: [MERGED] Pre-commit hooks

Post by adrianinsaval »

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
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [MERGED] Pre-commit hooks

Post by openBrain »

adrianinsaval 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
GH API
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: [MERGED] Pre-commit hooks

Post by adrianinsaval »

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
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [MERGED] Pre-commit hooks

Post by openBrain »

There is no pagination. This is a hard limit. ;)
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: [MERGED] Pre-commit hooks

Post by adrianinsaval »

openBrain wrote: Tue May 30, 2023 5:36 am There is no pagination. This is a hard limit. ;)
what API are you using? what I read said up to 300 changes per page but up to 3000 total. Also, we can't we just use a git command to get the changed files?
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [MERGED] Pre-commit hooks

Post by openBrain »

adrianinsaval wrote: Tue May 30, 2023 3:15 pm what API are you using?
The compare API between 2 SHA.
what I read said up to 300 changes per page but up to 3000 total.
https://docs.github.com/en/repositories ... iff-limits
Also, we can't we just use a git command to get the changed files?
Sure we can but today in Prepare step we haven't a clone of the repo, what I tried to avoid to add.

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.
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: [MERGED] Pre-commit hooks

Post by adrianinsaval »

I expect Lint does have a clone of the repo?
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [MERGED] Pre-commit hooks

Post by openBrain »

adrianinsaval wrote: Tue May 30, 2023 5:26 pm I expect Lint does have a clone of the repo?
Yes it already has one because Clang-tidy needs it.
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: [MERGED] Pre-commit hooks

Post by adrianinsaval »

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
Post Reply