Details
In native/cpp/CommoonCpp/NativeModules/CommCoreModule.cpp add #include "../../../native_rust_library/lib.rs.h" and in the same file in function getAllMessages add printf("%s\n", generate_device_id(DeviceType::KEYSERVER).c_str()); and see that after launching ios from XCode a string subjet to "^(ks|mobile|web):[a-zA-Z0-9]{64}$" is logged to the XCode console. I tested this launching
a phisical device as for some reason I cannot launch the simulator from XCode.
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/native_rust_library/src/crypto_tools.rs | ||
|---|---|---|
| 17 ↗ | (On Diff #17354) | I couldn't find a nice way of using DEVICE_ID_CHAR_LENGTH for defining this regex (I mean sth like "^(ks|mobile|web):[a-zA-Z0-9]{${DEVICE_ID_CHAR_LENGTH}}$"). The best way I found is using const_format create, but I'm not sure we want do introduce another dependency for something like this... |
| native/native_rust_library/src/lib.rs | ||
|---|---|---|
| 14 ↗ | (On Diff #17354) | Is using pub use here necessary? For the purpose of generating C++ bindings for this rust function use is enough. |
| 37 ↗ | (On Diff #17354) | Declaration outside of extern "Rust" scope is essential here since C++ would not be able to understand this type. Context is here: https://cxx.rs/shared.html. I am leaving this comment in case future reviewers get confused here. |
| 80 ↗ | (On Diff #17354) | I am not requesting it but in my opinion it would be nice to use some source of separation here like a comment line: //crypto_tools or something. Unfortunately we cannot create separate module for each function set we want to generate C++ bindings for, so we could at least provide "logical" separation with comments. |
| native/native_rust_library/src/crypto_tools.rs | ||
|---|---|---|
| 17 ↗ | (On Diff #17357) | If we can separate stuff related to tests from application code then I think we should do so. |
| native/native_rust_library/src/crypto_tools.rs | ||
|---|---|---|
| 17 ↗ | (On Diff #17357) | Originally this was in mod tests, but because of the need to keep this in sync with other files (as explained in the comment above the constant) I've put these constants that relate to some application logic next to each other, like in the other files. In rust code this is just related to tests, but in other places this regex is actually used. Eg. in services/tunnelbroker/src/Tools/Tools.cpp DEVICEID_FORMAT_REGEX from services/tunnelbroker/src/Constants.h is used validate device id passed to tunnelbroker. |
The code from D4754 had to be moved to our general rust library. As suggested in comments on D4754, the enum has been renamed to DeviceType.
Does that mean that the plan is to abandon that diff and land only this one? I don't think this is a good idea. Usually, we should update the diff so that the discussion is preserved. Also, moving already accepted code makes it necessary to review it again - it's hard to tell which parts are new and which are moved.