[Rpm-maint] [rpm-software-management/rpm] rpmte: Move dependencies into an unordered_map (PR #3538)
Panu Matilainen
notifications at github.com
Thu Feb 6 07:54:38 UTC 2025
@pmatilai commented on this pull request.
> @@ -85,22 +77,22 @@ struct rpmte_s {
rpmfs fs;
};
+/* Does not include RPMTAG_NAME as it needs special handling */
So, "ugly" is not helpful or constructive in the slightest. Sorry about that. I also shouldn't have merged this.
We ended up with a version that's like a classic caricature compromise - the worst of both worlds: a one-purpose internal data structure that doesn't contain the data necessary to use it for initialization, that logic is far away inside a loop and none of that makes any sense at all. And, the internal data structure even leaks to the ABI.
When you got an idle moment, please submit a PR that changes this either to your original version with everything in the code logic, that was ten times saner than the final version. Or my suggestion where everything is in the data structure.
Somehow this PR went from ok to a mess during the review process with (to me at least) the worst version getting merged. There's bound to be some takeaways from this. One of them being simply avoid review and decisions when sleep deprived (which I was yesterday), but also other stuff. I need to think about this some.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3538#discussion_r1944258458
You are receiving this because you are subscribed to this thread.
Message ID: <rpm-software-management/rpm/pull/3538/review/2597907743 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20250205/d10c7921/attachment.htm>
More information about the Rpm-maint
mailing list