Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
warnings were added to move the code to auth/Cryto.
But Have you raised an PR for movement?
Or any existing PR that has done movement.
I believe class AES_256_CTR needed to be moved .. but not done yet
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.
@amitkumar50 Please read the PR description above for the PR details for the movement.
First of all, adding warnings like this is not a good practise and the TODO should be specified in a tracker. Not within the code itself.
This PR is created as the PR #14498 takes too long to get merged.
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.
@amitkumar50
In addition to my previous comment, it is ridiculous to purposefully create warnings for the TODO comments in the code.
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.
maybe @aclamk remembers why the #warnings, I vaguely remember it might have been @liewegas that asked to do it. Anyhow, it'd be better to revert 326a16d.
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.
@yehudasa Yeah, that may be true. But it's been annoying nowadays as it appears in every other PR builds.
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.
@joscollin the advantage if you revert the commit is we have the warning retained as comment and helps us trace back git histories better