Skip to content

Minor script cleanups#7602

Closed
a1346054 wants to merge 5 commits intocurl:masterfrom
a1346054:master
Closed

Minor script cleanups#7602
a1346054 wants to merge 5 commits intocurl:masterfrom
a1346054:master

Conversation

@a1346054
Copy link
Contributor

@a1346054 a1346054 commented Aug 21, 2021

No description provided.

@ghost
Copy link

ghost commented Aug 21, 2021 •
edited by ghost
Loading

Congratulations 🎉. DeepCode analyzed your code in 2.534 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

👉 The DeepCode service and API will be deprecated in August, 2021. Here is the information how to migrate. Thank you for using DeepCode 🙏 ❤️ !

If you are using our plugins, you might be interested in their successors: Snyk's JetBrains plugin and Snyk's VS Code plugin.

@bagder bagder changed the title Minor cleanups Minor script cleanups Aug 21, 2021
@bagder
Copy link
Member

bagder commented Aug 21, 2021

I think the style changes for the shell scripts are unnecessary. I don't think they increase readability.

bagder added a commit that referenced this pull request Aug 21, 2021
@bagder
man pages: remove trailing whitespaces
7c86367 
Extended test 1173 (via the manpage-syntax.pl script) to detect and warn
for them.

Ref: #7602
Reported-by: a1346054
@bagder bagder mentioned this pull request Aug 21, 2021
Closed
@a1346054
Copy link
Contributor Author

a1346054 commented Aug 21, 2021

I'm taking advice from http://mywiki.wooledge.org/BashFAQ/082 and https://github.com/koalaman/shellcheck/wiki/SC2006 as well as other places.

bagder added a commit that referenced this pull request Aug 21, 2021
@bagder
man pages: remove trailing whitespaces
4e53b94 
Extended test 1173 (via the manpage-syntax.pl script) to detect and warn
for them.

Ref: #7602
Reported-by: a1346054 on github
Closes #7604
@a1346054
chore(perl): invoke perl through /usr/bin/env
f4c1301
@a1346054
style(maketgz): unify coding style with rest of repository
adabef1 
This introduces no changes in functionality, it only silences some
shellcheck warnings.
@a1346054
chore(MacOSX-Framework): invoke bash through /usr/bin/env
194b71e 
This makes sure that the correct bash is used on macOS, instead of the
hardcoded ancient one that lives in /bin/bash.
@a1346054
style(MacOSX-Framework): unify coding style with rest of repository
6e1ae91 
This introduces no changes in functionality, it only silences some
shellcheck warnings and makes the code more robust.
@a1346054
chore: be explicit about using 'python3' instead of 'python'
2d8d78a 
This fixes running tests in virtualenvs (or on distros) that no longer
have a symlink from python to python2 or python3.
@bagder
Copy link
Member

bagder commented Aug 23, 2021

I still don't agree with the code style commits and I think you should've done them in a separate PR as they're not the same change. I will go ahead and merge the shebang-fixes.

@bagder bagder closed this in 5458e6b Aug 23, 2021
bagder pushed a commit that referenced this pull request Aug 23, 2021
@a1346054 @bagder
tests: be explicit about using 'python3' instead of 'python'
ec91b5a 
This fixes running tests in virtualenvs (or on distros) that no longer
have a symlink from python to python2 or python3.

Closes #7602
@a1346054
Copy link
Contributor Author

a1346054 commented Aug 31, 2021

I reworked the patches to make fewer changes https://github.com/a1346054/curl/tree/fixes

I would appreciate any review feedback on how to proceed, whether I should make a new PR or leave things as they are.

@mback2k mback2k mentioned this pull request Nov 1, 2021
Closed
mback2k added a commit to mback2k/curl that referenced this pull request Mar 5, 2022
@mback2k
tests/*.py: support Python 2 again while preferring Python 3
9924e14 
This makes it possible to use the Python-based test servers
again in legacy environments which do not have modern Python.

Install Python package impacket in relevant CI workflows.

Follow up to curl#7602 which broke Python 2 support
Follow up to curl#7935 which fixed impacket support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@a1346054 @bagder