tests: Migrate create_slide.list.sh into cargo xtask function. #2957
Conversation
michael-kerscher
requested review from
djmitche,
mgeisler and
qwandor
as code owners
michael-kerscher
force-pushed
the
webtest-check-only-changed-slides-xtask
branch
2 times, most recently
from
ab1ac86 to
05381dc
Compare
This makes the functionality more reliable and more aligned to the skillset relevant for contributing in this repository. Also finally replaces the old way of running the script in the CI environment
michael-kerscher
force-pushed
the
webtest-check-only-changed-slides-xtask
branch
from
05381dc to
a8de18d
Compare
|
In the linked broken PR the previous shell script did something unexpected and worked with .js files and directories, which is wrong. |
| let mut cmd = Command::new("git"); | ||
| cmd.arg("diff") | ||
| .arg("--name-only") | ||
| .arg(format!("{}...", base_ref)) |
There was a problem hiding this comment.
this is one of the things I'm not 100% sure if this would create issues. This git diff should provide the list of filenames (--name-only) that was changed by this PR (over all commits). But I'm not sure about GITHUB_BASE_REF in various situations. I might need to use a more reliable way to do this
There was a problem hiding this comment.
GITHUB_BASE_REF might only be set in forked repositories (but not sure if this is the case), which would explain why I did not see issues with the previous shell script
michael-kerscher
force-pushed
the
webtest-check-only-changed-slides-xtask
branch
from
6ea837d to
a26ffc9
Compare
2 participants
The new xtask function makes the helper code
The shell script grew and was not readable for everyone anymore without deeper knowledge.
mitigates #2941 in a more reliable way but does still not fully fix the root cause