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

Miscellaneous Improvements #11

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Cgettys
Copy link

@Cgettys Cgettys commented Jan 3, 2025

I came across wrapped_mono through your article. It looks interesting, and I noticed some small fixes here and there I could easily contribute, so here I am, contributing them :).

Happy to split out any commits you want to handle individually, and feel free to ignore these. Tests still pass, except the proc macro doctests, which I never got to work on my system and did not make any related changes to.

The commit messages give a good overview, but to summarize:

  • Expand .gitignore to not include Jetbrains IDE files that don't belong being checked in (to prevent me from accidentally checking in said files)
  • Some formatting / wording tweaks in the readme
  • Some typos in traits and feature names (InteropRecive -> InteropReceive, referneced_objects -> referenced_objects
  • Fix warnings about unexpected feature old_gc_unsafe by using feature = "old_gc_unsafe" - recent change to make this warn, there was a blog post somewhere explaining.. https://doc.rust-lang.org/rustc/check-cfg.html is relevant here as well
  • Propagate write! failures via ? instead of silently producing the wrong generated code (this is the only functional change in the PR that I'm aware of)
  • Remove a few unnecessary mut keywords
  • Some minor assert!(a == b) to assert_eq!(a, b) and likewise for != to assert_ne

You might want to consider making src/lib.rs doctest embedded from readme.md, like rust-lang/rfcs#1990

@Cgettys Cgettys marked this pull request as ready for review January 3, 2025 02:47
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.

1 participant