[Rpm-maint] [rpm-software-management/rpm] Isolate module state to enable running in multiple subinterpreters (PR #3808)

[Michal Domonkos]

@dmnks requested changes on this pull request.

The effort that went into splitting the commits is very much appreciated. That said, I feel like it's a bit too granular for a patchset that we could merge into master. As a general rule, we want each commit to be self-contained, i.e. to compile successfully & pass the test-suite. It's crucial for future bisect-abiilty.

Are those intermediate steps really necessary in order to be recorded in the git history? If not, would you be willing to squash (some of) them?

For instance, the static `PyTypeObject*` declarations are removed in the first couple of commits and their usages are only fixed later. I think they should all be in a single commit since they belong to the same logical change.

Some commits also miss the *why*, e.g. the one titled `python rpmfile: Use the type allocation function`.

A couple of minor style issues I've noticed:

1. `Python module isolation: Remove statically stored types`: the subject line isn't separated by an empty line (breaking one-line git log output)
2. `Python module isolation: Visit PyObject* members in tp_visit hooks`: extra leading space in the subject



-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3808#pullrequestreview-2932570131
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/3808/review/2932570131 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20250616/dc221b28/attachment.htm>