Conversation
| SingleModule(PathBuf), | ||
| MultiModule(BTreeMap<String, PathBuf>), | ||
| SingleModule(String), | ||
| MultiModule(HashMap<String, String>), |
There was a problem hiding this comment.
Check if using a HashMap here doesn't impact buildtime determinism.
There was a problem hiding this comment.
That's a good point. I think it's likely best to just stick to BTreeMap.
There was a problem hiding this comment.
nanoserde doesn't support BTreeMap unfortunately, that's why I changed it.
but yes logically it should be a BTreeMap here. though the builds are rather non-deterministic anyway so probably quite fine. but should comment it
There was a problem hiding this comment.
Looks like it would be pretty easy to add BTreeMap to nanoserde if we wanted to, just saying. But yeah we can stick to hashmap while we evaluate.
There was a problem hiding this comment.
nanoserde has a closed issue on this here: not-fl3/nanoserde#7
Apparently the owner of the library prefers to use a proxy in this case
[...] instead of adding all possible containers into the library. "
Might be worthwhile to just open a PR over there if the additional buildtime determinism is worth it.
|
I think I just need to clean up some clean up some code here wrt to the file IO handling and remove some unwraps and we can get this in! |
4 participants
Part of #858