Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ext/net): add op_network_interfaces #12964

Merged
merged 1 commit into from
Dec 22, 2021

Conversation

bnoordhuis
Copy link
Contributor

Add an op to list the network interfaces on the system.

Prep work for #8137 and os.networkInterfaces() Node compat in std.

Refs #17850.


Opening this as a WIP because I'd like feedback on what permissions to check for.

@bartlomieju
Copy link
Member

Opening this as a WIP because I'd like feedback on what permissions to check for.

IMO this should be the same check as for Deno.hostname() (unstable API) which requires full --allow-env access.

Add an op to list the network interfaces on the system.

Prep work for denoland#8137 and `os.networkInterfaces()` Node compat in std.

Refs denoland/deno_std#1436.
@bnoordhuis bnoordhuis force-pushed the op_network_interfaces branch from fe5a91a to 60e5736 Compare December 22, 2021 21:42
@bnoordhuis bnoordhuis marked this pull request as ready for review December 22, 2021 21:43
@bnoordhuis
Copy link
Contributor Author

Done. I didn't add tests because it's not exposed as a public API yet but I'll make std/node exercise it.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@bnoordhuis bnoordhuis merged commit 1678690 into denoland:main Dec 22, 2021
@bnoordhuis bnoordhuis deleted the op_network_interfaces branch December 22, 2021 23:00
@kt3k
Copy link
Member

kt3k commented Jan 20, 2022

@bnoordhuis Have you started public API part of this work? If not, I'd like to continue the rest of the work

@bnoordhuis
Copy link
Contributor Author

@kt3k Only minimally so please feel free to have a go at 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.

3 participants