Skip to content
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

Readability change - srp_verifier.txt #24237

Closed
wants to merge 1 commit into from
Closed

Readability change - srp_verifier.txt #24237

wants to merge 1 commit into from

Conversation

hrtarsia
Copy link
Contributor

@hrtarsia hrtarsia commented Apr 22, 2024 •
edited
Loading

I enclosed each of the values (I, g, and N) in single quotes to increase readability and lessen any confusion for the readers.

Checklist
  • documentation is added or updated

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Apr 22, 2024
@hrtarsia
Copy link
Contributor Author

This is one of my PR requests for the Software Engineering Assignment with @bbbrumley.

@bbbrumley
Copy link
Contributor

Hi!

This looks fine to me, but the maintainers are still gonna want the CLA line in the actual commit message; see the documentation here, particularly the part about "To amend a missing .."

@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Apr 23, 2024
@hrtarsia
Readability change - srp_verifier.txt
d6b622f 
Fix grammar in srp_verifier.txt

CLA: trivial
@hrtarsia
Copy link
Contributor Author

I believe I have properly added the CLA line now, thank you professor.

@bbbrumley
Copy link
Contributor

I believe I have properly added the CLA line now, thank you professor.

Looks great ty! Let's see what the maintainers say

mattcaswell
mattcaswell approved these changes Apr 23, 2024
View reviewed changes
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Agree trivial

@mattcaswell mattcaswell added branch: master Merge to master branch approval: review pending This pull request needs review by a committer triaged: documentation The issue/pr deals with documentation (errors) tests: exempted The PR is exempt from requirements for testing labels Apr 23, 2024
t8m
t8m approved these changes Apr 23, 2024
View reviewed changes
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

OK with CLA: trivial

@t8m t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Apr 23, 2024
@shahsb
Copy link
Contributor

shahsb commented Apr 23, 2024 •
edited
Loading

Hence forth, Please add the issue number in the commit message.
Fixes # {no}
Thanks!

@hrtarsia
Copy link
Contributor Author

Will do! Thank you.

@t8m
Copy link
Member

t8m commented Apr 24, 2024

Hence forth, Please add the issue number in the commit message.
Fixes # {no}
Thanks!

I do not think this PR fixes any concrete issue. It is just a documentation cleanup. So there is no point in adding the Fixes # annotation.

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@t8m t8m added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Apr 25, 2024
@t8m
Copy link
Member

t8m commented Apr 25, 2024

Merged to the master branch. Thank you for your contribution.

@t8m t8m closed this Apr 25, 2024
openssl-machine pushed a commit that referenced this pull request Apr 25, 2024
@hrtarsia @t8m
Fix grammar in srp_verifier.txt
5d218b0 
CLA: trivial

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #24237)
jvdsn pushed a commit to jvdsn/openssl that referenced this pull request Jun 3, 2024
@hrtarsia @jvdsn
Fix grammar in srp_verifier.txt
4808fd5 
CLA: trivial

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#24237)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t8m t8m t8m approved these changes

@mattcaswell mattcaswell mattcaswell approved these changes

@shahsb shahsb shahsb approved these changes

Assignees
No one assigned
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch tests: exempted The PR is exempt from requirements for testing triaged: documentation The issue/pr deals with documentation (errors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@hrtarsia @bbbrumley @shahsb @t8m @openssl-machine @mattcaswell