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

Drop shebang from upload_doc.sh #408

Merged
merged 1 commit into from
Jan 24, 2024
Merged

Drop shebang from upload_doc.sh #408

merged 1 commit into from
Jan 24, 2024

Conversation

cottsay
Copy link
Contributor

@cottsay cottsay commented Jan 24, 2024

🦟 Bug fix

Summary

This file isn't directly executable in any of its forms:

  • When checked into the repository, it is a template
  • When installed by gz-cmake, it is still a template
  • When expanded in downstream packages, it isn't explicitly given executable permission, requiring an explicit interpreter invocation

Additionally, the usage for this script instructs the user to explicitly provide the interpreter. To avoid misleading users and linters into believing that this file should be executed directly, drop the (currently unused) shebang.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

This file isn't directly executable in any of its forms:
* When checked into the repository, it is a template
* When installed by gz-cmake, it is still a template
* When expanded in downstream packages, it isn't explicitly given
  executable permission, requiring an explicit interpreter invocation

Additionally, the usage for this script instructs the user to explicitly
provide the interpreter. To avoid misleading users and linters into
believing that this file should be executed directly, drop the
(currently unused) shebang.

Signed-off-by: Scott K Logan <[email protected]>
@github-actions github-actions bot added 🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic labels Jan 24, 2024
@azeey
Copy link
Contributor

azeey commented Jan 24, 2024

I think it's fine to drop the shebang. Not many people use this script directly. It's mainly used by CI to generate and upload documentation, and there, the interpreter is explicitly provided (see https://github.com/gazebosim/docs/blob/baafa500efa6b29ef2ad99f929bbba616435794d/tools/scripts/build_gz.sh#L33)

@azeey azeey merged commit a29a26f into gazebosim:gz-cmake3 Jan 24, 2024
9 checks passed
@cottsay cottsay deleted the drop-shebang branch January 24, 2024 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants