Repository navigation
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the declaration inventory and technical-debt scope. The 46 entries are explicitly import-resolution stubs that throw, do not claim recovered signatures or behavior, and are recorded in the per-library debt files. The report describes an independent exact-name/NID audit against current exports and additions, a live overlap scan, and successful builds of all four affected library targets on Windows and WSL/Linux. Conventions, intake, eligibility, labels and the progress report pass. Hosted Linux/Windows product jobs were skipped; the reported targeted platform builds are the available compile evidence.
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the declarations and debt entries. The 46 aliases use the source import names, distinct no-parameter nid_no_patch wrappers that throw, and documentation that explicitly leaves signatures/behavior unknown. No tests were added that merely check symbol presence. The author reports an exact NID audit against main and current open PR additions, no overlap from pr_overlap.py, and Windows/WSL builds for all four affected libraries. Hosted eligibility, intake, conventions, labels and report checks pass; Linux/Windows/platform build jobs are skipped. I did not run local tests or independently reproduce the NID audit.
MarcZX10
left a comment
There was a problem hiding this comment.
The prior review’s evidence supports the scope: 46 source-inventory declarations, throwing no-parameter stubs with unknown ABI/behavior recorded in debt, and reported Windows/WSL builds of all four libraries. The current branch is based on a6dcea9, while upstream main is ab347d1. Please rebase and rerun the exact import-name/NID uniqueness audit and pr_overlap.py against the current main and live open PR set, then rerun conventions and the four library target builds. The existing hosted policy checks pass, but product builds are skipped; I did not run local tests or independently reproduce the prior NID audit.
6b33719 to
446a5a5
Compare
|
Understood. The 46-entry audit and four-library build reports were for an older base. A current-main rebase and fresh exact import-name/NID audit, live overlap scan, conventions check, and affected-library builds remain necessary before merge. I have not done that update and am not presenting the prior checks as current. |
9cbb204 to
7404d95
Compare
|
|
|
You were right: these declarations also passed readable export names as literal NIDs. I changed all 46 APS5_EXPORT first arguments to the corresponding 11-character NIDs, verified against the pinned aerolib database (including the 43 libkernel_web names). The sources establish the names, but not complete callable prototypes for every entry, so the no-parameter throwing wrappers and ABI-debt notes remain explicit import-resolution placeholders. |
|
The bot could not compare #9970 because it conflicts with the base, so I checked these shared files manually. #9485 adds only |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
a7b58f3 to
03f4285
Compare
|
Relevant warning: comparisons of the shared RegMgr and Http2 export/debt files were skipped because the other PR branches conflict with base. I have not treated this as a clean overlap result. I am checking the function/NID sets in #10153, #10167, #10534 and #10536 against this PR before claiming those overlaps are clear. |
|
Manual review complete for the skipped exports. PR 10153 adds named RegMgr functions whose computed NIDs differ from this PR's HGxgXwGSAzQ. PR 10167 changes existing Http2 implementations without adding an export for this NID. PR 10534 adds no export declarations. The seven named Http2 exports in PR 10536 compute to NIDs distinct from this PR's AS45QoYHjc4. I found no duplicate function or NID in these shared export files. The checker warning remains relevant for conflicting branches and shared debt hunks, so this manual result is limited to the listed export overlaps. |
|
I’ve closed my PRs because, based on my personal experience, I’ve decided I will no longer contribute to or update them. This decision is about my experience with the public community and is not directed at the maintainers or active reviewers, whose time and work I appreciate. Thank you for reviewing and discussing my contributions. I wish the project and its maintainers well. |
1 similar comment
|
I’ve closed my PRs because, based on my personal experience, I’ve decided I will no longer contribute to or update them. This decision is about my experience with the public community and is not directed at the maintainers or active reviewers, whose time and work I appreciate. Thank you for reviewing and discussing my contributions. I wish the project and its maintainers well. |
What
Adds 46 pending PS5 3.20 import declarations from PS5-3.20_Libs: 43 in
libkernel_web, one inlibSceHttp2, one inlibSceNKWebCdlgInjectedBundle, and one inlibSceRegMgr. Each declaration uses the repository's no-parameter unknown-signature convention and callsNotImplemented_nid_no_patch. These declarations support import resolution only; their prototypes, ABI, and behavior are unverified. Technical-debt entries document the unknown cases.The independent NID audit recomputed each selected NID from its source import name, checked exact library/NID pairs and global NID uniqueness against upstream
main, the prior 1,088-PR audit, and current additions from PRs #9470–#9484. No selected pair or NID is already present.tools/pr_overlap.pyalso reports no overlap with the live open PR set.Tested
python -I tools/check_conventions.py --base upstream/main --head HEAD— passedgit diff --check upstream/main...HEAD— passedtools/pr_overlap.pyagainst the live open PR set — no overlaplibkernel_web,libSceHttp2,libSceNKWebCdlgInjectedBundle, andlibSceRegMgrcompiled and linked successfullyChecklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); no silent stubs were added.mdfiles or images added to the repository (the technical-debt entries are required documentation)