On Wed, Jun 27, 2018 at 5:02 AM, Adam Gashlin <agash...@mozilla.com> wrote: > * Already vendored crates > Can I assume any crates we have already in mozilla-central are ok to use?
Seems like a reasonable assumption. > * Updates > I need winapi 0.3.5 for BITS support, currently third_party/rust/winapi is > 0.3.4. There should be no problem updating it, but should I have this > reviewed by the folks who originally vendored it into mozilla-central? In my opinion, it should be enough for _someone_ qualified to review code in the area of Windows integration to review the diff. > * New crates > I'd like to use the windows-service crate, which seems well written and has > few dependencies, but the first 0.1.0 release was just a few weeks ago. I'd > like to have that reviewed at least as carefully as my own code, > particularly given how much unsafety there is, but where do I draw the > line? For instance, it depends on "widestring", which is small and has been > around for a while but isn't widely used, should I have that reviewed > internally as well? Is popularity a reasonable measure? In principle, all code landing in m-c needs to be reviewed, but sometimes the reviewer may rubber-stamp code instead of truly reviewing it carefully. All the newly-vendored code should be part of the review request and then it's up to the reviewer to decide if it's appropriate to look at some code less carefully because there are other indicators of quality. As for widestring specifically, a cursory look at the code suggests that it's a quality crate and should have no trouble passing review. It is also small enough that it should be actually feasible to review it instead of rubber-stamping it. (For Mozilla-developed code that is on a performance-sensitive path, there exists encoding_rs::mem (particularly https://docs.rs/encoding_rs/0.8.4/encoding_rs/mem/fn.convert_str_to_utf16.html and https://docs.rs/encoding_rs/0.8.4/encoding_rs/mem/fn.convert_utf16_to_str.html), which doesn't provide the ergonomics that widestring provides but provides faster (SIMD-accelerated on our tier-1 CPU architectures and aarch64, which is on path to tier-1) conversions for long (16 code units or longer) strings containing mostly ASCII code points. An update service probably isn't performance-sensitive in this way. I'm mentioning this to generate awareness generally on the topic of UTF-16 conversions in m-c Rust code.) -- Henri Sivonen hsivo...@mozilla.com _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
