| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
WalkthroughThe time_ns function in the time standard library module was changed to be compiled unconditionally for all target operating systems, including wasi, by removing the previous conditional compilation attribute. The function implementation itself remains unchanged. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review details Configuration used: .coderabbit.yml Reviewing files that changed from the base of the PR and between d5793e0 and 69f6423. 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ Share 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Sorry, something went wrong.
| import functools | ||
| import getpass | ||
| import opcode | ||
| import logging |
There was a problem hiding this comment.
The CI fails on wasi because logging uses time.time_ns which is not available on wasi (yet).
How should I approach this? should I patch this here so we won't crash if this raises an AttributeError, or should this PR be blocked until we implement time.time_ns for wasi?
Sorry, something went wrong.
There was a problem hiding this comment.
That's up to the situation. In this case, we already have _time for wasi, so it must be easy to add time_ns in wasi. (it will be inaccurate though) So I will add time_ns in this case.
If it was not easy, I would try to patch logging a little bit to ignore time_ns or to use time
Sorry, something went wrong.
| Ok(()) | ||
| } | ||
|
|
||
| #[cfg(not(target_os = "wasi"))] |
There was a problem hiding this comment.
This is what I ended up doing. This line was inserted 4 years ago. maybe things changed since then? now it passes for wasi 🤷 .
Usure if this is a correct fix for the issue
Sorry, something went wrong.
There was a problem hiding this comment.
looks good
Sorry, something went wrong.
|
@coderabbitai generate unit tests |
Sorry, something went wrong.
|
An unexpected error occurred during Unit Test Generation. |
Sorry, something went wrong.
|
✅ Actions performed
Unit Test Generation started... |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you so much. This is a big step to support other tests
Sorry, something went wrong.
| Ok(()) | ||
| } | ||
|
|
||
| #[cfg(not(target_os = "wasi"))] |
There was a problem hiding this comment.
looks good
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary by CodeRabbit