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

Misc bash script refactoring #1595

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Misc bash script refactoring #1595

merged 1 commit into from
Jun 12, 2024

Conversation

edmorley
Copy link
Member

This:

  • adjusts the way scripts are sourced to always use the full path from the root of the buildpack, so shellcheck can find the scripts without needing the shellcheck source=... directive
  • stops leaking some internal env vars to user-facing subprocesses
  • removes some duplicate sourcing of scripts and use of shopt
  • removes some low value historic code comments (that make the classic mistake of repeating what the code does, and not adding anything new, such as why)

This has been split out of a later PR to ease review.

@edmorley edmorley self-assigned this Jun 12, 2024
This:
- adjusts the way scripts are sourced to always use the full
  path from the root of the buildpack, so shellcheck can find
  the scripts without needing the `shellcheck source=...`
  directive
- stops leaking some internal env vars to user-facing subprocesses
- removes some duplicate sourcing of scripts and use of `shopt`
- removes some low value historic code comments (that make the
  classic mistake of repeating what the code does, and not adding
  anything new, such as why)

This has been split out of a later PR to ease review.
@edmorley edmorley marked this pull request as ready for review June 12, 2024 09:15
@edmorley edmorley requested a review from a team as a code owner June 12, 2024 09:15
@edmorley edmorley enabled auto-merge (squash) June 12, 2024 09:18
@edmorley edmorley merged commit 055f8c6 into main Jun 12, 2024
6 checks passed
@edmorley edmorley deleted the misc-refactoring branch June 12, 2024 16:10
edmorley added a commit that referenced this pull request Jun 13, 2024
* Switches to using `git ls-files` instead of a hardcoded list of scripts to
  lint (shellcheck doesn't support recursively scanning for scripts itself).
  This means a few files that were not previously linted now are, so need
  fixes.
* Adds a `.shellcheckrc` which is used to enable optional shellcheck
  checks via `enable=all`. Most of these checks either already passed
  or have been fixed in this PR - however, a few others need a closer
  look so have been disabled for now to reduce the size of this PR.
* As part of doing this I discovered an NLTK bug with setting `NLTK_DATA`
  after #1595, which has now been fixed. Since that change was not yet
  released, I've not mentioned the fix here in the changelog.
@heroku-linguist heroku-linguist bot mentioned this pull request Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants