GeezerVison

I'm replying here since I dislike comments lost on a commit, I don't think that many people will notice it.

I'm fine with people doing reviews after I merged changes (I like them!).

@AlexWaygood wrote:

I don't like this at all :(

Requiring that an argparse.Namespace object be passed to parse_file() feels like we're coupling the parse_file() function to the specific context in which it's currently called. A Namespace object is essentially unstructured data; we should be parsing the Namespace object in run_clinic(), and passing only the specific variables we need to parse_file(). Apart from anything else, this makes the code much less type-safe.

Free feel to change it in a follow-up PR. Your rationale makes sense and I don't have a strong preference for passing arguments here. Or I can do it if you prefer.

I'm still struggling a lot with this code base and I tried to write the minimum working change without touching too many parts of the code. The problem is that I'm not used with the AC design, so I'm not sure if it's the right place to add code...

I'd appreciate if you'd give me a day or so to review Argument Clinic PRs @vstinner — I didn't see the review request for this one until it was already merged :)

I understand your frustration. I didn't expect any review at all in fact. I'm always happy to get reviews, even if it's after I merge my change.

Today I wanted to push multiple changes to unblock my work on removing private functions: see PR #108499 which kind-of the end of my "PR serie".

I'm frequently frustrated by the fact that GitHub doesn't let me create a "patch serie" (multiple PRs depending on each others). So I have to merge my changes one by one, but I'm always bitten by a CI which dislike my change, then I have to wait 30 min for the next issue... On the AC changes, I didn't expect that make check-c-globals would fail for example.

I wanted to push the basis on AC work to add #include and to support the limited C API. For following changes, it should easier to work on multiple PRs at the same time.

I'm glad that other people are now working on AC. Adding #include and supporting the limited C API was a long awaited feature for me, I started working on that in 2020 :-)

Follow-up PR: #108504

I'm replying here since I dislike comments lost on a commit, I don't think that many people will notice it.

Oops, I didn't mean to add a comment on the commit — I meant to comment on the PR. Thanks!

I understand your frustration. I didn't expect any review at all in fact.

Come on, you know that both Alex and I are active on a daily basis on CPython ;)

I do not think that it is correct. args is a Python tuple object here.

Oh ok, so I suppose that it can be removed.