-
Notifications
You must be signed in to change notification settings - Fork 61
Only include files modified by the PR in tarball #1119
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
Only include files modified by the PR in tarball #1119
Conversation
Instance
|
bot: build repo:eessi.io-2023.06-software instance:eessi-bot-mc-aws arch:x86_64/amd/zen2 |
Updates by the bot instance
|
New job on instance
|
bot: build repo:eessi.io-2023.06-software instance:eessi-bot-mc-aws arch:x86_64/amd/zen2 |
This also was not triggered in this case:
New job on instance
|
bot: build repo:eessi.io-2023.06-software instance:eessi-bot-mc-aws arch:x86_64/amd/zen2 |
This seems to work as expected, in the Slurm logs I see:
New job on instance
|
NOTE This PR does not need to be deployed |
@@ -8,6 +8,35 @@ display_help() { | |||
echo " -h | --help - display this usage information" | |||
} | |||
|
|||
file_changed_in_pr() { | |||
local full_path="$1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also check that full_path
is not empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that kind of done at line 16?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test -f
(without argument) exits with zero, so I'd say no
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, test -f ""
does fail with non-zero exit code, so it's fine as is I guess, don't want to nitpick here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You had me sweating but
ocaisa@~$ export full_path="$PWD/file"; [[ -f "$full_path" ]] || echo Fail
Fail
ocaisa@~$ touch file
ocaisa@~$ export full_path="$PWD/file"; [[ -f "$full_path" ]] || echo Fail
ocaisa@~$ export full_path=""; [[ -f "$full_path" ]] || echo Fail
Fail
ocaisa@~$ export full_path= ; [[ -f "$full_path" ]] || echo Fail
Fail
ocaisa@~$ unset full_path
ocaisa@~$ [[ -f "$full_path" ]] || echo Fail
Fail
so the check below is indeed enough
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
One more test run to rule out utter breakage (I'm aware no deploy is needed here): bot: build repo:eessi.io-2023.06-software instance:eessi-bot-mc-aws arch:x86_64/amd/zen2 |
New job on instance
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks fine, if there's any fallout we'll deal with it in a follow-up PR
5ca8e88
into
EESSI:2023.06-software.eessi.io
No description provided.