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

fix: update deno_core to latest to work better with rust 1.78 #3890

Merged
merged 3 commits into from
Jun 10, 2024

Conversation

rubenfiszel
Copy link
Contributor

@rubenfiszel rubenfiszel commented Jun 10, 2024

🚀 This description was created by Ellipsis for commit f895ae1

Summary:

This PR updates dependencies, refines Deno integration, and enhances runtime setup in the windmill-worker module, with specific improvements in permissions handling and snapshot management.

Key points:

  • Updated dependencies in Cargo.toml for backend and windmill-worker.
  • Modified Rust files in windmill-worker and js_eval.rs to update Deno permissions handling.
  • Adjusted JavaScript runtime setup in windmill-worker to align with updated Deno modules.
  • Enhanced snapshot creation and handling in windill-worker build script.
  • Integrated new Deno network and TLS functionalities in the JavaScript runtime setup.

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 393fa99 in 2 minutes and 55 seconds

More details
  • Looked at 1329 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. backend/Cargo.toml:170
  • Draft comment:
    Ensure that the deno_ast version in Cargo.toml matches the updated version in Cargo.lock to maintain consistency and avoid potential dependency resolution issues.
deno_ast = { version = "0.39.1", features = ["transpiling"] }
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_DI6NkFc0wwkIj3ay


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

cloudflare-workers-and-pages bot commented Jun 10, 2024

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: f895ae1
Status:⚡️  Build in progress...

View logs

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on a301b25 in 2 minutes and 1 seconds

More details
  • Looked at 68 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_XukT4PwIXAecFOym


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -882,6 +922,55 @@ multiline template`";
Ok(())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The test function test_eval_timeout_bug initializes a Deno runtime but does not perform any operations or assertions with it, making it ineffective as a test. Consider adding relevant JavaScript code execution and assertions to validate the runtime behavior.

@rubenfiszel rubenfiszel changed the title all fix: update deno_core to latest to work better with rust 1.78 Jun 10, 2024
@rubenfiszel rubenfiszel merged commit ec248dd into main Jun 10, 2024
2 of 3 checks passed
@rubenfiszel rubenfiszel deleted the rf/denoUpgrade branch June 10, 2024 16:31
@github-actions github-actions bot locked and limited conversation to collaborators Jun 10, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on f895ae1 in 1 minute and 59 seconds

More details
  • Looked at 139 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. backend/windmill-worker/src/js_eval.rs:779
  • Draft comment:
    The removal of Deno network and TLS module imports needs verification against updated Deno module changes to ensure continued functionality.
  • Reason this comment was not posted:
    Confidence of 40% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_hV4oEbxNQ7bD6ySX


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant