Skip to content

Java: Add query for RSA without OAEP#9982

Merged
joefarebrother merged 8 commits intogithub:mainfrom
joefarebrother:rsa-without-oaep
Aug 23, 2022
Merged

Java: Add query for RSA without OAEP#9982
joefarebrother merged 8 commits intogithub:mainfrom
joefarebrother:rsa-without-oaep

Conversation

@joefarebrother
Copy link
Copy Markdown
Contributor

No description provided.

@atorralba
Copy link
Copy Markdown
Contributor

Looks good to me (only needs rebasing to fix the failing test I think).

Asking for a docs review.

@atorralba atorralba added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Aug 17, 2022
@saritai
Copy link
Copy Markdown
Contributor

saritai commented Aug 17, 2022

👋 Docs content first responder here! Adding this to our review board so a writer can review it! 🙇‍♀️

@mchammer01 mchammer01 self-requested a review August 18, 2022 07:34
mchammer01
mchammer01 previously approved these changes Aug 18, 2022
Copy link
Copy Markdown
Contributor

@mchammer01 mchammer01 left a comment

Choose a reason for hiding this comment

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

@joefarebrother - this LGTM ✨
I've added a few minor comments or suggestions for your consideration.

Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com>
@sidshank
Copy link
Copy Markdown
Contributor

@joefarebrother What're the next steps for this PR? 🤠 I see that your latest commit applies suggestions from @mchammer01 - Does this PR need to go through another doc review pass? Are any additional reviews required apart from the docs review?

@joefarebrother joefarebrother merged commit ac79866 into github:main Aug 23, 2022
@atorralba
Copy link
Copy Markdown
Contributor

@sidshank I think we were waiting for performance evaluation in DCA, but since @aschackmull approved it, it means we're good in that sense and @joefarebrother went ahead with the merge 😄

@aschackmull
Copy link
Copy Markdown
Contributor

Luckily the query looks sufficiently innocent that I'm not too worried about its performance. But if you were waiting for dca then you could just have waited on the merge - I approved the PR, as the QL looks fine, so you could merge whenever you were ready to do so.

@atorralba
Copy link
Copy Markdown
Contributor

Well, I assumed that was the case because of the DCA runs linked to this PR, but @joefarebrother will know for sure.

@joefarebrother
Copy link
Copy Markdown
Contributor Author

I was trying to do a DCA run but it wasn't working (now that I think about it I probably just needed to rebase); merged because it was approved and that it seemed unlikely to have a performance issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Java ready-for-doc-review This PR requires and is ready for review from the GitHub docs team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants