Skip to content

fix: Various improvements.#6

Merged
cjihrig merged 8 commits into
cjihrig:ffifrom
ShogunPanda:ffi
Mar 25, 2026
Merged

fix: Various improvements.#6
cjihrig merged 8 commits into
cjihrig:ffifrom
ShogunPanda:ffi

Conversation

@ShogunPanda

Copy link
Copy Markdown

No description provided.

@ShogunPanda ShogunPanda force-pushed the ffi branch 5 times, most recently from 40db774 to 570ba85 Compare March 20, 2026 05:59

@cjihrig cjihrig left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a number of comments. I didn't make it through everything, but it seems like maybe some rogue AI made some inadvisable changes.

Comment thread lib/internal/bootstrap/realm.js Outdated
Comment thread lib/internal/process/permission.js Outdated
Comment thread lib/ffi.js
Comment thread src/ffi/data.cc Outdated
Comment thread src/ffi/data.cc Outdated
Comment thread test/ffi/fixture_library/ffi_test_library.def Outdated
Comment thread test/ffi/ffi-test-common.js Outdated
Comment thread test/ffi/ffi-test-common.js Outdated
Comment thread test/ffi/ffi-test-common.js Outdated
Comment thread test/ffi/ffi.status
@ShogunPanda

Copy link
Copy Markdown
Author

@cjihrig I reverted many unneeded changes. Do you mind checking again?

@cjihrig

cjihrig commented Mar 23, 2026

Copy link
Copy Markdown
Owner

Thanks. I left a comment about exception handling in C++. I will take a more in depth look again later.

@ShogunPanda

Copy link
Copy Markdown
Author

Thanks, I didn't know about exception handling. I replaced all of them with simple returns.

Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
os.environ.get('VSCMD_ARG_TGT_ARCH'),
os.environ.get('Platform'),
os.environ.get('PROCESSOR_ARCHITECTURE'),
platform.machine(),

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know Python, but is there any reason not to only rely on platform.machine()?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea, I trusted AI on this :)

Comment thread doc/api/cli.md Outdated
Comment thread doc/api/cli.md Outdated
Comment thread doc/api/cli.md
Comment thread doc/api/ffi.md Outdated
Comment thread test/parallel/test-process-env-allowed-flags-are-documented.js Outdated
Comment thread test/parallel/test-process-get-builtin.mjs
Comment thread test/parallel/test-process-versions.js Outdated
Comment thread test/parallel/test-process-versions.js Outdated
Comment thread test/parallel/test-require-resolve.js Outdated
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
@ShogunPanda

Copy link
Copy Markdown
Author

@cjihrig Finally everything is green. This should be ready to be merged!

@cjihrig cjihrig left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks a lot.

@cjihrig cjihrig merged commit f46b6e0 into cjihrig:ffi Mar 25, 2026
26 checks passed
@ShogunPanda ShogunPanda deleted the ffi branch March 26, 2026 04:50
ShogunPanda added a commit that referenced this pull request Apr 6, 2026
* src,deps,test: various improvements

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* doc,lib: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

---------

Signed-off-by: Paolo Insogna <paolo@cowtech.it>
ShogunPanda added a commit that referenced this pull request Apr 11, 2026
* src,deps,test: various improvements

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps,test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* doc,lib: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* deps: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

* test: fixed failures

Signed-off-by: Paolo Insogna <paolo@cowtech.it>

---------

Signed-off-by: Paolo Insogna <paolo@cowtech.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants