Skip to content

Conversation

@JADSN1894
Copy link

@JADSN1894 JADSN1894 commented Oct 7, 2025

Goals

  1. Update devcontainer settings image to rust:latest (optional)
  2. Update the dependencies
  3. Update the edition to 2024 at root Cargo.toml workspace
  4. Update the resolver to 3 at root Cargo.toml workspace
  5. Fix unsafe blocks to resolve compilation warnings/errors for WebAssembly taragets
  6. Sorting depencies order in Cargo.toml sections

All these changes is to turn the code up-to-date based on stable channel to reduce technical debt and security risks and more organized between all project packages.

@pchickey
Copy link
Contributor

pchickey commented Oct 7, 2025

Did you intend to make this PR upstream? Currently there are a lot of changes committed throughout that I don't think you meant to upstream, e.g. devcontainer settings, sorting order in Cargo.toml sections, etc

@JADSN1894
Copy link
Author

What is a path to update the dependencies to make this PR upstream?

@tschneidereit
Copy link
Member

@JADSN1894 as a start, you should add a clear description of what the PR does, and why these changes are needed. Without that, it's very hard for reviewers to evaluate the implementation.

@JADSN1894
Copy link
Author

@JADSN1894 as a start, you should add a clear description of what the PR does, and why these changes are needed. Without that, it's very hard for reviewers to evaluate the implementation.

Thanks for reply. I write a short goals description.

@alexcrichton
Copy link
Member

@JADSN1894 at over 2000 lines of miscellaneous changes this PR is not going to merge as-is. Your goals make sense to me, but please split this up into separate PRs that are more focused as opposed to performing large codebase refactorings trying to achieve multiple goals at the same time.

@cfallin
Copy link
Member

cfallin commented Oct 8, 2025

@JADSN1894 to offer some more constructive feedback: it would be good for you to read through the diff you've created and make sure everything makes sense and is meant to be included. In particular, I note that you seem to have done a global find-replace for gen to r#gen (a Rust 2024 change?) but that was applied everywhere, including e.g. in literal strings for build paths and in the README (e.g. see here), which is surely incorrect. For first-time contributors with nontrivial PRs in particular, this kind of thing is a signal about code quality that you'll want to be careful about.

@JADSN1894
Copy link
Author

Is it better make this PR a draft and try again another way, like split this up into separate PRs?
Thanks for patience.

@alexcrichton
Copy link
Member

Splitting up would be preferable yeah, if that's managable.

@JADSN1894
Copy link
Author

Doubts

  1. How to test in devcontainer or locally and pass the CI / Build wit-bindgen (<arch>) (pull_request)
  2. I don't know how to pass the CI / Check (pull_request), always fail becasuse the file diff crates/guest-rust/src/rt/wit_bindgen_cabi_realloc.rs

@tschneidereit
Copy link
Member

Hey @JADSN1894, we really appreciate your work towards improving things here. We also all have fairly limited capacity to help you work through understanding how the system works, unfortunately.

I would recommend that you take a step back and go back to the unmodified main branch, apply just one small change, build it locally and run tests, and make sure you understand how that all works. Then you can open a PR for that change and take on the next one. And where you run into failures like the one you're mentioning here, I'd recommend looking at the CI job's definition, see how it works, and maybe find the PR in which it was introduced to see the description. (Which for me personally is what I'd have to do to answer questions about it, too.)

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.

5 participants