Skip to content

Improvement | Dispose RandomNumberGenerator object #1478

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Mar 9, 2022
Merged

Improvement | Dispose RandomNumberGenerator object #1478

merged 7 commits into from
Mar 9, 2022

Conversation

EngRajabi
Copy link
Contributor

@EngRajabi EngRajabi commented Jan 18, 2022 •
edited
Loading

Improvement | dispose RandomNumberGenerator object after generate randome bytes
fixes #1494

DavoudEshtehari
DavoudEshtehari reviewed Jan 28, 2022
View reviewed changes
Copy link
Contributor

@DavoudEshtehari DavoudEshtehari left a comment

Choose a reason for hiding this comment

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

Nice catch. Could you take care of the AkvStoreProviderVerifyFunctionWithInvalidSignature too?

@EngRajabi
Copy link
Contributor Author

Thanks for checking the code. I did not change the tests because dispose in the tests did not have a specific function.
I do not know why builds fail. Interestingly, a timeout error is received. Can you help me?

@EngRajabi
Copy link
Contributor Author

@DavoudEshtehari @johnnypham @JRahnama @Kaur-Parminder thanks

@JRahnama
Copy link
Contributor

I just reran the failed pipelines, LGTM. that was nice catch @EngRajabi, well done.

@EngRajabi EngRajabi mentioned this pull request Jan 29, 2022
Closed
mokarchi
mokarchi approved these changes Jan 30, 2022
View reviewed changes
Copy link

@mokarchi mokarchi left a comment

Choose a reason for hiding this comment

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

That’s great @EngRajabi

@EngRajabi
Copy link
Contributor Author

@DavoudEshtehari @Kaur-Parminder please check. thanks

@DavoudEshtehari
Copy link
Contributor

Thanks for checking the code. I did not change the tests because dispose in the tests did not have a specific function. I do not know why builds fail. Interestingly, a timeout error is received. Can you help me?

I believe you've updated the tests. I'd appreciate it if you could include the requested improvement in this PR for the sake of uniformity.

@JRahnama JRahnama changed the title dispose RandomNumberGenerator object Improvement | Dispose RandomNumberGenerator object Feb 1, 2022
@EngRajabi
Copy link
Contributor Author

Hello
Thank you for reviewing the request. @DavoudEshtehari @Kaur-Parminder

@johnnypham
Copy link
Contributor

Hello Thank you for reviewing the request. @DavoudEshtehari @Kaur-Parminder

If you can add one more dispose in AkvStoreProviderVerifyFunctionWithInvalidSignature, we can get this merged.

@EngRajabi
Copy link
Contributor Author

Hello Thank you for reviewing the request. @DavoudEshtehari @Kaur-Parminder

If you can add one more dispose in AkvStoreProviderVerifyFunctionWithInvalidSignature, we can get this merged.

This part is inside the test. Is it that important?

@EngRajabi
Copy link
Contributor Author

@johnnypham done! please merge

@DavoudEshtehari DavoudEshtehari added this to the 5.0.0-preview1 milestone Mar 8, 2022
@DavoudEshtehari DavoudEshtehari added the Enhancement 💡 Issues that are feature requests for the drivers we maintain. label Mar 8, 2022
@EngRajabi

This comment was marked as off-topic.

Sign in to view
@DavoudEshtehari DavoudEshtehari merged commit 43296f9 into dotnet:main Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kaur-Parminder Kaur-Parminder Awaiting requested review from Kaur-Parminder

4 more reviewers

@johnnypham johnnypham johnnypham approved these changes

@JRahnama JRahnama JRahnama approved these changes

@mokarchi mokarchi mokarchi approved these changes

@DavoudEshtehari DavoudEshtehari DavoudEshtehari approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Enhancement 💡 Issues that are feature requests for the drivers we maintain.
Projects
None yet
Milestone
5.0.0-preview1
Development

Successfully merging this pull request may close these issues.

Improvement | dispose RandomNumberGenerator object #1478
6 participants
@EngRajabi @JRahnama @DavoudEshtehari @johnnypham @mokarchi @Kaur-Parminder