Skip to content

rid nearly all use of ExceptionRef in zig - #15100

Merged
dylan-conway merged 6 commits into
mainfrom
nektro-patch-36822
Nov 13, 2024
Merged

rid nearly all use of ExceptionRef in zig#15100
dylan-conway merged 6 commits into
mainfrom
nektro-patch-36822

Conversation

@nektro

@nektro nektro commented Nov 12, 2024

Copy link
Copy Markdown
Contributor

progress towards zig JSValue stability and safety in #159
many followups to come

@robobun

robobun commented Nov 12, 2024

Copy link
Copy Markdown
Collaborator

@nektro, your commit 20fc424 has 3 failures in #5952:

  • test/js/bun/http/serve.test.ts - 1 failing on 🍎 x64
  • test/js/web/fetch/fetch.tls.test.ts - 1 failing on 🍎 aarch64
  • test/cli/run/require-cache.test.ts - 1 failing on 🪟 x64
  • @nektro
    nektro marked this pull request as ready for review November 12, 2024 08:52
    @nektro
    nektro requested a review from dylan-conway November 12, 2024 08:53
    Comment thread src/bun.js/api/bun/socket.zig Outdated
    if (!globalObject.hasException() and exception != null) {
    globalObject.throwValue(exception.?.value());
    }
    const handlers = Handlers.fromJS(globalObject, socket_obj) catch return .zero;

    Copy link
    Copy Markdown
    Member

    Choose a reason for hiding this comment

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

    nice

    Comment thread src/bun.js/api/BunObject.zig Outdated
    Comment thread src/bun.js/api/BunObject.zig Outdated
    Comment thread src/bun.js/api/JSTranspiler.zig Outdated
    Comment thread src/bun.js/api/BunObject.zig Outdated
    Comment thread src/bun.js/node/util/parse_args.zig Outdated
    Comment thread src/bun.js/node/util/parse_args.zig Outdated
    Comment thread src/bun.js/node/util/parse_args.zig Outdated
    Comment thread src/bun.js/node/node_fs.zig

    @paperclover paperclover left a comment

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

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

    amazing

    Comment thread src/fd.zig Outdated
    Comment thread src/options.zig Outdated
    Comment thread src/options.zig Outdated

    @dylan-conway dylan-conway left a comment

    Copy link
    Copy Markdown
    Member

    Choose a reason for hiding this comment

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

    Looks great. I left a few suggestions. Important ones are in options.zig

    @dylan-conway
    dylan-conway merged commit f8979b0 into main Nov 13, 2024
    @dylan-conway
    dylan-conway deleted the nektro-patch-36822 branch November 13, 2024 23:23
    snoglobe pushed a commit that referenced this pull request Nov 18, 2024
    Co-authored-by: nektro <nektro@users.noreply.github.com>
    snoglobe pushed a commit that referenced this pull request Nov 21, 2024
    Co-authored-by: nektro <nektro@users.noreply.github.com>
    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.

    4 participants