Skip to content

Fix typos found by codespell #20910

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
Closed
DimitriPapadopoulos wants to merge 2 commits into from
Closed

Fix typos found by codespell #20910

DimitriPapadopoulos wants to merge 2 commits into from

Conversation

@DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented May 9, 2023 •
edited
Loading

CLA: trivial

While I have modified many files, this is a trivial merge request in the sense that this is codespell fixing typos semi-automatically.

I have not modified the sacrosanct LICENSE.txt.

I have modified CHANGES.md and NEWS.md, some projects would rather not modify these files when they are directly generated from SCV messages, but I looks like it is not the case here.

I have mostly modified documentation and code comments. There are two occurrences of misspelling a variable name, but the impact is very local, inside a source file.

Might overlap #16712. If you merge #16712, I will rebase to fix remaining typos.

Checklist
  • documentation is added or updated
  • tests are added or updated

@DimitriPapadopoulos DimitriPapadopoulos mentioned this pull request May 9, 2023
Closed
DimitriPapadopoulos
DimitriPapadopoulos commented May 9, 2023
View reviewed changes
tom-cosgrove-arm
tom-cosgrove-arm reviewed May 9, 2023
View reviewed changes
tom-cosgrove-arm
tom-cosgrove-arm reviewed May 9, 2023
View reviewed changes
tom-cosgrove-arm
tom-cosgrove-arm reviewed May 9, 2023
View reviewed changes
tom-cosgrove-arm
tom-cosgrove-arm reviewed May 9, 2023
View reviewed changes
@tom-cosgrove-arm
Copy link
Contributor

tom-cosgrove-arm commented May 9, 2023

As a single PR I am concerned that this goes beyond the scope of CLA: trivial

@DimitriPapadopoulos DimitriPapadopoulos force-pushed the codespell branch 4 times, most recently from 25bcdd1 to 7142654 Compare May 9, 2023 09:47
@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 9, 2023

I will sign the CLA if requested.

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label May 9, 2023
@DimitriPapadopoulos DimitriPapadopoulos mentioned this pull request May 9, 2023
Closed
2 tasks
mattcaswell
mattcaswell requested changes May 9, 2023
View reviewed changes
@mattcaswell
Copy link
Member

mattcaswell commented May 9, 2023

While all the changes in this are individually trivial, collectively there is a lot of change going on. I think a CLA would be a good idea.

Please submit one:
https://www.openssl.org/policies/cla.html

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 9, 2023

ICLA signed and sent by mail.

levitte
levitte reviewed May 9, 2023
View reviewed changes
@t8m t8m added hold: cla required The contributor needs to submit a license agreement cla: trivial One of the commits is marked as 'CLA: trivial' triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly branch: master Applies to master branch labels May 9, 2023
@t8m
Copy link
Member

t8m commented May 9, 2023

Could you please also split out the changes in the manual pages to a separate PR as I think we should backport these fixes to 3.1 and 3.0 branches.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 9, 2023 •
edited
Loading

@t8m So you suggest a separate PR for doc/man* only, don't you? Or all of doc?

@paulidale
Copy link
Contributor

paulidale commented May 9, 2023

Just doc/man* will be enough.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 10, 2023 •
edited
Loading

Closed / reopened to remove the hold: cla required flag as instructed.

@DimitriPapadopoulos DimitriPapadopoulos mentioned this pull request May 10, 2023
Closed
2 tasks
@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 10, 2023

Changes in doc/man* have been moved to #20924.

t8m
t8m requested changes May 10, 2023
View reviewed changes
@t8m t8m added approval: review pending This pull request needs review by a committer tests: exempted The PR is exempt from requirements for testing labels May 10, 2023
@paulidale
Copy link
Contributor

paulidale commented May 15, 2023

Please don't force push updates. We're prefer fixup commits.
With a force push, it's more difficult to see what the (usually small) changes were.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 15, 2023 •
edited
Loading

I have to force push anyway when rebasing:

Patches should be as current as possible; expect to have to rebase often.

But I get it, rebasing against master is one thing, force pushing my own changes is another thing.

@t8m
Copy link
Member

t8m commented May 15, 2023

Rebase only if there is a conflict that needs to be resolved. Otherwise there is no point in rebasing except for cases where the PR is getting really old - i.e. more than half a year or so.

slontis
slontis reviewed Jun 13, 2023
View reviewed changes
@DimitriPapadopoulos
Fix typos found by codespell
9d3ddb0 
Typos in doc/man* will be fixed in a different commit.
@DimitriPapadopoulos
Revert UNPARSABLE
481fa14
@paulidale paulidale 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 Jun 13, 2023
@openssl-machine
Copy link
Collaborator

openssl-machine commented Jun 15, 2023

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.

@paulidale
Copy link
Contributor

paulidale commented Jun 15, 2023

Merged, thanks for the contribution.

@paulidale paulidale closed this Jun 15, 2023
openssl-machine pushed a commit that referenced this pull request Jun 15, 2023
@DimitriPapadopoulos @paulidale
Fix typos found by codespell
eb4129e 
Typos in doc/man* will be fixed in a different commit.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #20910)
@DimitriPapadopoulos DimitriPapadopoulos deleted the codespell branch June 15, 2023 06:24
This was referenced Jul 17, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slontis slontis slontis left review comments

@t8m t8m t8m approved these changes

@paulidale paulidale paulidale approved these changes

@mattcaswell mattcaswell Awaiting requested review from mattcaswell

@tom-cosgrove-arm tom-cosgrove-arm Awaiting requested review from tom-cosgrove-arm

@levitte levitte Awaiting requested review from levitte

Assignees

No one assigned

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch cla: trivial One of the commits is marked as 'CLA: trivial' severity: fips change The pull request changes FIPS provider sources tests: exempted The PR is exempt from requirements for testing triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@DimitriPapadopoulos @tom-cosgrove-arm @mattcaswell @t8m @paulidale @openssl-machine @levitte @slontis