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

(maint) update scripts/release/release.sh #199

Merged
merged 1 commit into from
Aug 4, 2021
Merged

(maint) update scripts/release/release.sh #199

merged 1 commit into from
Aug 4, 2021

Conversation

vladdoster
Copy link
Contributor

@vladdoster vladdoster commented Jul 3, 2021

  • use $(...) over `...` (backticks) because it is the legacy syntax required by only the very oldest of non-POSIX-compatible bourne-shells
  • remove un-used START_DIR variable
  • use printf bringing it up to date with the rest of the script and honors the \n without needing a -e flag.

- use  $(...) over `...` (backticks) because it is the legacy syntax required by only the very oldest of non-POSIX-compatible bourne-shells
- remove un-used `START_DIR` variable
- use `printf` bringing it up to date with the rest of the script and honors the `\n` without needing a `-e` flag.
@vladdoster
Copy link
Contributor Author

@jptoto @aareet

Does this repo take external PRs?

Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

Hi @vladdoster
Sorry for the delay in reviewing your PR.

The changes you propose look reasonable to me but may I ask you what motivated the PR since all these changes don't seem to fix any particular bug - unless I'm mistaken?

I should mention that we use pretty much the same script in a number of other repositories: https://github.com/search?q=org%3Ahashicorp+getTargetVersion&type=code

@radeksimko radeksimko merged commit 9792c20 into hashicorp:main Aug 4, 2021
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