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 script cleanups #7602

Closed
wants to merge 5 commits into from
Closed

Minor script cleanups #7602

wants to merge 5 commits into from

Conversation

@a1346054
Copy link
Contributor

@a1346054 a1346054 commented Aug 21, 2021

No description provided.

@deepcode-ci-bot
Copy link

@deepcode-ci-bot deepcode-ci-bot bot commented Aug 21, 2021

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 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 issue Aug 21, 2021
Extended test 1173 (via the manpage-syntax.pl script) to detect and warn
for them.

Ref: #7602
Reported-by: a1346054
@a1346054
Copy link
Contributor Author

@a1346054 a1346054 commented Aug 21, 2021

bagder added a commit that referenced this issue Aug 21, 2021
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 added 5 commits Aug 21, 2021
This introduces no changes in functionality, it only silences some
shellcheck warnings.
This makes sure that the correct bash is used on macOS, instead of the
hardcoded ancient one that lives in /bin/bash.
This introduces no changes in functionality, it only silences some
shellcheck warnings and makes the code more robust.
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 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 added a commit that referenced this issue Aug 23, 2021
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 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants