From: Phillip Wood <phil@crinan.ddns.net>
To: Patrick Steinhardt <ps@pks.im>,
Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 5/5] ci: add support for GitLab CI
Date: Fri, 27 Oct 2023 11:22:35 +0100 [thread overview]
Message-ID: <d62b2e38-6e24-4661-b51d-2ecc59eae256@crinan.ddns.net> (raw)
In-Reply-To: <ZTtyHScKkMg4qnMH@tanuki>
On 27/10/2023 09:17, Patrick Steinhardt wrote:
>>> + CI_BRANCH="$CI_COMMIT_REF_NAME"
>>> + CI_COMMIT="$CI_COMMIT_SHA"
>>>
>> assignments need no quoting to prevent word splitting.
>> repeats below.
>>
>>> + case "$CI_JOB_IMAGE" in
>>>
>> ... as does the selector in case statements.
>
> True, but I'm simply matching the coding style in this script.
I think it is quite common for us to quote variables when it isn't
strictly necessary as it makes it clear to anyone reading the script
that there is no word splitting going on and ensures that we don't start
splitting the variable if the contents changes in the future.
>>> --- a/ci/print-test-failures.sh
>>> +++ b/ci/print-test-failures.sh
>>> @@ -51,6 +51,12 @@ do
>>> tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
>>> continue
>>> ;;
>>> + gitlab-ci)
>>> + mkdir -p failed-test-artifacts
>>> + cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
>>> + tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
>>>
>> you're just following the precedent, but imo it's more legible to quote the
>> entire string, not just the variable expansion. the code doesn't even agree
>> with itself, as the line directly above apparently agrees with me.
>>
>> regards
>
> Yeah, as you say, this is another case where I follow precedent. I
> honestly don't quite care which way we go in this case.
Yes, if you're following existing practice I don't think this is worth
worrying about.
Best Wishes
Phillip
next prev parent reply other threads:[~2023-10-27 10:29 UTC|newest]
Thread overview: 101+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-26 7:59 [PATCH 0/5] ci: add GitLab CI definition Patrick Steinhardt
2023-10-26 8:00 ` [PATCH 1/5] ci: reorder definitions for grouping functions Patrick Steinhardt
2023-10-26 8:26 ` Oswald Buddenhagen
2023-10-27 8:17 ` Patrick Steinhardt
2023-10-26 8:00 ` [PATCH 2/5] ci: make grouping setup more generic Patrick Steinhardt
2023-10-26 8:00 ` [PATCH 3/5] ci: group installation of Docker dependencies Patrick Steinhardt
2023-10-26 8:34 ` Oswald Buddenhagen
2023-10-27 8:17 ` Patrick Steinhardt
2023-10-26 8:00 ` [PATCH 4/5] ci: split out logic to set up failed test artifacts Patrick Steinhardt
2023-10-26 8:35 ` Oswald Buddenhagen
2023-11-03 22:35 ` Christian Couder
2023-11-06 7:16 ` Patrick Steinhardt
2023-10-26 8:00 ` [PATCH 5/5] ci: add support for GitLab CI Patrick Steinhardt
2023-10-26 9:07 ` Oswald Buddenhagen
2023-10-27 8:17 ` Patrick Steinhardt
2023-10-27 10:22 ` Phillip Wood [this message]
2023-10-27 10:43 ` Oswald Buddenhagen
2023-10-27 14:32 ` Phillip Wood
2023-10-27 17:47 ` Oswald Buddenhagen
2023-10-30 9:49 ` Phillip Wood
2023-10-30 14:04 ` Dragan Simic
2023-10-27 10:49 ` Oswald Buddenhagen
2023-10-27 11:11 ` Patrick Steinhardt
2023-10-27 9:25 ` [PATCH v2 0/5] ci: add GitLab CI definition Patrick Steinhardt
2023-10-27 9:25 ` [PATCH v2 1/5] ci: reorder definitions for grouping functions Patrick Steinhardt
2023-10-27 9:25 ` [PATCH v2 2/5] ci: make grouping setup more generic Patrick Steinhardt
2023-10-27 9:25 ` [PATCH v2 3/5] ci: group installation of Docker dependencies Patrick Steinhardt
2023-10-27 9:25 ` [PATCH v2 4/5] ci: split out logic to set up failed test artifacts Patrick Steinhardt
2023-10-27 9:25 ` [PATCH v2 5/5] ci: add support for GitLab CI Patrick Steinhardt
2023-10-27 10:19 ` Phillip Wood
2023-10-27 11:19 ` Patrick Steinhardt
2023-10-27 11:57 ` Patrick Steinhardt
2023-10-27 13:02 ` Phillip Wood
2023-10-29 16:13 ` Phillip Wood
2023-10-30 10:46 ` Patrick Steinhardt
2023-10-29 16:27 ` Phillip Wood
2023-10-30 10:45 ` Patrick Steinhardt
2023-10-30 0:22 ` Junio C Hamano
2023-10-27 11:01 ` Oswald Buddenhagen
2023-10-27 13:17 ` Phillip Wood
2023-10-27 15:53 ` Oswald Buddenhagen
2023-10-31 19:36 ` Jeff King
2023-11-01 3:33 ` Junio C Hamano
2023-10-30 12:14 ` [PATCH v3 0/8] ci: add GitLab CI definition Patrick Steinhardt
2023-10-30 12:14 ` [PATCH v3 1/8] ci: reorder definitions for grouping functions Patrick Steinhardt
2023-10-30 12:14 ` [PATCH v3 2/8] ci: make grouping setup more generic Patrick Steinhardt
2023-10-30 12:14 ` [PATCH v3 3/8] ci: group installation of Docker dependencies Patrick Steinhardt
2023-10-30 12:14 ` [PATCH v3 4/8] ci: split out logic to set up failed test artifacts Patrick Steinhardt
2023-10-30 12:15 ` [PATCH v3 5/8] ci: unify setup of some environment variables Patrick Steinhardt
2023-10-30 15:09 ` Phillip Wood
2023-10-30 15:19 ` Patrick Steinhardt
2023-10-30 18:22 ` Dragan Simic
2023-10-30 12:15 ` [PATCH v3 6/8] ci: squelch warnings when testing with unusable Git repo Patrick Steinhardt
2023-10-30 12:15 ` [PATCH v3 7/8] ci: install test dependencies for linux-musl Patrick Steinhardt
2023-10-30 12:47 ` Patrick Steinhardt
2023-10-30 13:22 ` Patrick Steinhardt
2023-10-30 15:13 ` Phillip Wood
2023-10-30 15:23 ` Patrick Steinhardt
2023-10-30 16:09 ` Phillip Wood
2023-10-30 12:15 ` [PATCH v3 8/8] ci: add support for GitLab CI Patrick Steinhardt
2023-10-30 15:46 ` [PATCH 0/5] ci: add GitLab CI definition Taylor Blau
2023-10-31 7:46 ` Patrick Steinhardt
2023-10-31 19:12 ` Taylor Blau
2023-11-01 0:15 ` Junio C Hamano
2023-11-01 11:56 ` Patrick Steinhardt
2023-10-31 9:04 ` [PATCH v4 0/8] " Patrick Steinhardt
2023-10-31 9:04 ` [PATCH v4 1/8] ci: reorder definitions for grouping functions Patrick Steinhardt
2023-10-31 9:04 ` [PATCH v4 2/8] ci: make grouping setup more generic Patrick Steinhardt
2023-10-31 9:04 ` [PATCH v4 3/8] ci: group installation of Docker dependencies Patrick Steinhardt
2023-10-31 9:04 ` [PATCH v4 4/8] ci: split out logic to set up failed test artifacts Patrick Steinhardt
2023-10-31 9:04 ` [PATCH v4 5/8] ci: unify setup of some environment variables Patrick Steinhardt
2023-10-31 17:06 ` Victoria Dye
2023-11-01 3:14 ` Junio C Hamano
2023-11-01 11:44 ` Patrick Steinhardt
2023-10-31 9:05 ` [PATCH v4 6/8] ci: squelch warnings when testing with unusable Git repo Patrick Steinhardt
2023-10-31 9:05 ` [PATCH v4 7/8] ci: install test dependencies for linux-musl Patrick Steinhardt
2023-10-31 9:05 ` [PATCH v4 8/8] ci: add support for GitLab CI Patrick Steinhardt
2023-10-31 17:47 ` Victoria Dye
2023-11-01 11:44 ` Patrick Steinhardt
2023-10-31 18:22 ` [PATCH v4 0/8] ci: add GitLab CI definition Victoria Dye
2023-11-01 3:22 ` Junio C Hamano
2023-11-01 11:44 ` Patrick Steinhardt
2023-11-01 13:02 ` [PATCH v5 0/8] ci: add support for GitLab CI Patrick Steinhardt
2023-11-01 13:02 ` [PATCH v5 1/8] ci: reorder definitions for grouping functions Patrick Steinhardt
2023-11-01 13:02 ` [PATCH v5 2/8] ci: make grouping setup more generic Patrick Steinhardt
2023-11-01 13:02 ` [PATCH v5 3/8] ci: group installation of Docker dependencies Patrick Steinhardt
2023-11-01 13:02 ` [PATCH v5 4/8] ci: split out logic to set up failed test artifacts Patrick Steinhardt
2023-11-01 13:03 ` [PATCH v5 5/8] ci: unify setup of some environment variables Patrick Steinhardt
2023-11-01 13:03 ` [PATCH v5 6/8] ci: squelch warnings when testing with unusable Git repo Patrick Steinhardt
2023-11-01 13:03 ` [PATCH v5 7/8] ci: install test dependencies for linux-musl Patrick Steinhardt
2023-11-01 13:03 ` [PATCH v5 8/8] ci: add support for GitLab CI Patrick Steinhardt
2023-11-09 8:05 ` [PATCH v6 0/8] ci: add GitLab CI definition Patrick Steinhardt
2023-11-09 8:05 ` [PATCH v6 1/8] ci: reorder definitions for grouping functions Patrick Steinhardt
2023-11-09 8:05 ` [PATCH v6 2/8] ci: make grouping setup more generic Patrick Steinhardt
2023-11-09 8:05 ` [PATCH v6 3/8] ci: group installation of Docker dependencies Patrick Steinhardt
2023-11-09 8:05 ` [PATCH v6 4/8] ci: split out logic to set up failed test artifacts Patrick Steinhardt
2023-11-09 8:05 ` [PATCH v6 5/8] ci: unify setup of some environment variables Patrick Steinhardt
2023-11-09 8:05 ` [PATCH v6 6/8] ci: squelch warnings when testing with unusable Git repo Patrick Steinhardt
2023-11-09 8:05 ` [PATCH v6 7/8] ci: install test dependencies for linux-musl Patrick Steinhardt
2023-11-09 8:05 ` [PATCH v6 8/8] ci: add support for GitLab CI Patrick Steinhardt
2023-11-09 10:06 ` [PATCH v6 0/8] ci: add GitLab CI definition Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d62b2e38-6e24-4661-b51d-2ecc59eae256@crinan.ddns.net \
--to=phil@crinan.ddns.net \
--cc=git@vger.kernel.org \
--cc=oswald.buddenhagen@gmx.de \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.