Skip to content

Conversation

@CoolSpy3
Copy link
Member

@CoolSpy3 CoolSpy3 commented Mar 20, 2023

To be tested

Copy link
Member

@brettle brettle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes seem fine but I recommend waiting until post-competition to avoid introducing new issues at this point. If there is an issue with thread-safety in the rev code, it may only occur occasionally and would thus be very difficult to debug. I also recommend reaching out to rev to find out if their code is supposed to be thread safe (unless you see that documented somewhere).

@CoolSpy3
Copy link
Member Author

I also recommend reaching out to rev to find out if their code is supposed to be thread safe (unless you see that documented somewhere).

I emailed Rev, and they report that "Regarding whether REVLib is thread-safe, it is intended to be thread-safe, yes!"

brettle
brettle previously approved these changes May 18, 2023
If a mock's reference was cleared between IS_REFERENCE_CLEARED.test and CLEAR_INVOCATIONS_ON_REFERENCED_MOCK.accept, the latter would throw an exception. By storing the referenced value in a member variable throughout both operations, the value cannot be null. This change should be tested before being merged into master
@CoolSpy3 CoolSpy3 added the Passed Testing This PR was successfully tested on a robot label Jun 14, 2023
@ProfessorAtomicManiac
Copy link
Contributor

I'm unfamiliar with how multithreading works so at the moment I'm not sure what to comment on. I trust that Ryan got everything down :)

@CoolSpy3 CoolSpy3 added enhancement New feature or request Non-Breaking Change This PR will introduce new backwards-compatible functionality labels Jun 30, 2023
@CoolSpy3 CoolSpy3 merged commit ab4f4e2 into master Jul 9, 2023
@CoolSpy3 CoolSpy3 deleted the async-periodic branch July 21, 2023 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Non-Breaking Change This PR will introduce new backwards-compatible functionality Passed Testing This PR was successfully tested on a robot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants