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.

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

Minor cleanups #16362

Closed
wants to merge 3 commits into from
Closed

Minor cleanups #16362

wants to merge 3 commits into from

Conversation

a1346054
Copy link
Contributor

Trivial spelling, whitespace, hashbang, and codestyle fixes.

Different tests may use unexpectedly different versions of perl,
depending on whether they hardcode the path to the perl executable or if
they resolve the path from the environment. This fixes it so that the
same perl is always used.

CLA: trivial
@levitte levitte added the cla: trivial One of the commits is marked as 'CLA: trivial' label Aug 19, 2021
config Outdated
@@ -6,5 +6,5 @@
# in the file LICENSE in the source distribution or at
# https://www.openssl.org/source/license.html

THERE=`dirname $0`
THERE=$(dirname $0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting question whether this should be done, or the greater portabality of backtick should be preserved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solaris 10 default shell might have an issue with `...` vs $(...) but I have no way of testing that.

@@ -36,16 +36,16 @@ cd include/openssl || exit 1
grep "$PAT" * | grep -v ERR_FATAL_ERROR | awk '{print $3;}' | sort -u >$X1
cd ../..

for F in `cat $X1` ; do
for F in $(cat $X1) ; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Same portability comment. I prefer to keep the backticks.

On the other hand, a case could be made to just remove this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with removal of files from the repository, if they are of no use anymore.

@richsalz
Copy link
Contributor

Congrats on the hashbang perl fixes!

@t8m t8m closed this Aug 23, 2021
@t8m t8m reopened this Aug 23, 2021
@t8m t8m added approval: otc review pending This pull request needs review by an OTC member branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly labels Aug 24, 2021
@t8m
Copy link
Member

t8m commented Aug 24, 2021

Please drop the shellcheck fix commit and I'll approve the PR.

@a1346054
Copy link
Contributor Author

Rebased without the shellcheck fixes.

@t8m
Copy link
Member

t8m commented Aug 24, 2021

I agree with CLA: trivial.

@t8m t8m added approval: review pending This pull request needs review by a committer and removed approval: otc review pending This pull request needs review by an OTC member labels Aug 24, 2021
Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

Agreed trivial.

@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 Sep 1, 2021
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Sep 2, 2021
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Sep 2, 2021
openssl-machine pushed a commit that referenced this pull request Sep 2, 2021
Different tests may use unexpectedly different versions of perl,
depending on whether they hardcode the path to the perl executable or if
they resolve the path from the environment. This fixes it so that the
same perl is always used.

Fix some trailing whitespace and spelling mistakes as well.

CLA: trivial

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #16362)
openssl-machine pushed a commit that referenced this pull request Sep 2, 2021
Different tests may use unexpectedly different versions of perl,
depending on whether they hardcode the path to the perl executable or if
they resolve the path from the environment. This fixes it so that the
same perl is always used.

Fix some trailing whitespace and spelling mistakes as well.

CLA: trivial

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #16362)

(cherry picked from commit 473664a)
@paulidale paulidale added the branch: 3.0 Merge to openssl-3.0 branch label Sep 2, 2021
@paulidale
Copy link
Contributor

Merged to master and 3.0.

@paulidale paulidale closed this Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch cla: trivial One of the commits is marked as 'CLA: trivial' triaged: bug The issue/pr is/fixes a bug triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants