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

rid nearly all use of ExceptionRef in zig #15100

Merged
merged 6 commits into from
Nov 13, 2024
Merged

Conversation

nektro
Copy link
Member

@nektro nektro commented Nov 12, 2024

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

@robobun
Copy link

robobun commented Nov 12, 2024

@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
    if (!globalObject.hasException() and exception != null) {
    globalObject.throwValue(exception.?.value());
    }
    const handlers = Handlers.fromJS(globalObject, socket_obj) catch return .zero;
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    nice

    Copy link
    Member

    @paperdave paperdave left a comment

    Choose a reason for hiding this comment

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

    amazing

    src/fd.zig Outdated Show resolved Hide resolved
    src/options.zig Outdated Show resolved Hide resolved
    src/options.zig Outdated Show resolved Hide resolved
    Copy link
    Collaborator

    @dylan-conway dylan-conway left a comment

    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
    22 of 25 checks passed
    @dylan-conway dylan-conway deleted the nektro-patch-36822 branch November 13, 2024 23:23
    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