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

A0-3310: Tidy up double quotes #1471

Merged
merged 5 commits into from
Nov 11, 2023

Conversation

ghost
Copy link

@ghost ghost commented Nov 2, 2023

Description

Removes unnecessary double quotes in the workflow files. Also, tidies up some variable names and make use of ${{ env.VAR }} everywhere, for the consistency and security.

Type of change

Please review carefully. We cannot break deployment workflows.

Checklist:

@ghost ghost self-assigned this Nov 2, 2023
@ghost ghost requested review from Marcin-Radecki and Kosiare November 2, 2023 07:07
Copy link
Contributor

@Marcin-Radecki Marcin-Radecki left a comment

Choose a reason for hiding this comment

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

While I agree with enclosing all github expressions in single quotes, I think it'll be better if we stay with double quotes in bash snippets. This is because, in general in bash it is better to use double quotes, from our bash style guide:

This prevents reinterpretation of all special characters within the quoted string — except $, ` (backquote), and \ (escape).
Keeping $ as a special character within double quotes permits referencing a quoted variable ("$variable"), that is, replacing the variable with its value.

so now if we stay in single quotes in bash snippets, and someone will add bash some variable ${var}, it will be directly interpreted as ${var} string

@mikolajgs mikolajgs assigned mikolajgs and unassigned ghost Nov 11, 2023
@mikolajgs mikolajgs added this pull request to the merge queue Nov 11, 2023
Merged via the queue into main with commit ec12db0 Nov 11, 2023
11 checks passed
@mikolajgs mikolajgs deleted the A0-3310-remove-unnecessary-double-quotes branch November 11, 2023 20:46
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.

3 participants