-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(dashmap): use stable dashmap #141
Conversation
Signed-off-by: Daniel Thompson-Yvetot <denjell@mailscript.com>
Signed-off-by: Daniel Thompson-Yvetot <denjell@mailscript.com>
The reason that I think this change (once accepted) would warrant a new release is because of the new macOS silicon (which I have not been able to test, but am terribly worried about). |
Signed-off-by: Daniel Thompson-Yvetot <denjell@mailscript.com>
riker-macros/Cargo.toml
Outdated
syn = { version ="0.15.39", features = ["parsing", "full", "extra-traits", "proc-macro"] } | ||
quote = "0.6.3" | ||
proc-macro2 = "0.4.4" | ||
syn = { version ="1.0", features = ["parsing", "full", "extra-traits", "proc-macro"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a pull request which fixes this. It shouldn't really matter, but I think it would be better to keep them separate and change those versions back to what they were earlier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure - I can revert that change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must have forgotten that I did that so late at night. I guess we can consider this as proof that the PR will also work with the updated riker-macros
as proposed in #136
Signed-off-by: Daniel Thompson-Yvetot <denjell@mailscript.com>
@nothingismagick , @hardliner66 and team. Merging this now and will prepare a new release. Thanks! |
This is basically the same as pr #132, just resolved the conflict. I ran pre-commit, all tests pass, all examples seem to work fine and no warnings are thrown on build. I tested it integrated in a project that works with the current release version, and this change continues to work properly and as expected (on macOS). I did remove a couple of the changes proposed by @BFrog because there is apparently no longer any need for the Dashmap iterator and no reason to submit code that has warnings. ;)
I guess it would probably be good to add a test to check this deadlock issue though, but I am not sure how to replicate that.
Closes #131
Closes #122