You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As one the maintainers of getrandom, I would advise against your getrandom_custom feature. Generally speaking, this feature is set by a user who is registering a custom handler (as explained in our docs) and not by a library that just calls getrandom::getrandom. The reason for this is because if a user enables the feature, but does not register a handler, they will get a confusing linker error.
It seems like this feature addition was done to reexport the register_custom_getrandom macro, but a user of nanorand-rs can already just register a handler by just directly depending on getrandom. It should also be noted that directly exporting our macro creates a dependency hazard, as the macro requires a function that returns getrandom::Error. So you would not be able to internally update your version of getrandom (say from 0.2 to 1.0) without having a breaking change yourself. Without this feature, you would be able to update getrandom without needing to release a breaking change in nanorand.
I couldn't find any discussion or PR about why 34ba243 was added. However, it seems like none of the released versions include it yet, so it could be reverted without too much issue.
All the other uses of getrandom's features seem reasonable to me. You could probably unconditionally enable the rdrand feature (even on non-x86 targets) as it has no effect on those targets. Keeping the js feature enabling separate is a good idea due to issues around feature unification and tkaitchuck/aHash#95 (comment)
The text was updated successfully, but these errors were encountered:
As one the maintainers of
getrandom
, I would advise against yourgetrandom_custom
feature. Generally speaking, this feature is set by a user who is registering a custom handler (as explained in our docs) and not by a library that just callsgetrandom::getrandom
. The reason for this is because if a user enables the feature, but does not register a handler, they will get a confusing linker error.It seems like this feature addition was done to reexport the
register_custom_getrandom
macro, but a user of nanorand-rs can already just register a handler by just directly depending ongetrandom
. It should also be noted that directly exporting our macro creates a dependency hazard, as the macro requires a function that returnsgetrandom::Error
. So you would not be able to internally update your version ofgetrandom
(say from0.2
to1.0
) without having a breaking change yourself. Without this feature, you would be able to updategetrandom
without needing to release a breaking change innanorand
.I couldn't find any discussion or PR about why 34ba243 was added. However, it seems like none of the released versions include it yet, so it could be reverted without too much issue.
All the other uses of
getrandom
's features seem reasonable to me. You could probably unconditionally enable therdrand
feature (even on non-x86 targets) as it has no effect on those targets. Keeping thejs
feature enabling separate is a good idea due to issues around feature unification and tkaitchuck/aHash#95 (comment)The text was updated successfully, but these errors were encountered: