Conversation
test.sh
Outdated
|
|
||
| if [ $# -gt 0 ]; then | ||
| files_to_process="$@" | ||
| if [[ "$1" == all ]]; then |
There was a problem hiding this comment.
| if [[ "$1" == all ]]; then | |
| if [[ "$1" == --all ]]; then |
There was a problem hiding this comment.
Maybe make --all the default because it requires no further input for the user.
test.sh
Outdated
| else | ||
| files_to_process=$all_markdown_files | ||
| # We only want to run tests for the notebooks we touched | ||
| files_to_process=$(git fetch origin main --depth=1; git diff origin/main --name-only tutorials | grep .md) |
There was a problem hiding this comment.
If the usage is --changed <branch> we can avoid hard-coding a specific branch name.
There was a problem hiding this comment.
Sounds good, and the we can hard-code origin and main on CI as those are set up there.
|
fyi: I'm stuck with the git logic a bit, to figure out a generic way for the arguments to pass on to git fetch/etc, as opposed to what we had to be run on CI only before. |
921f875 to
52287fb
Compare
|
I took this for a spin. Maybe we need two arguments: so that we can |
|
Yes, that's the one I converged on yesterday, I think I just sent myself down to unnecessary rabbit holes during the summit of automatically finding base branches for the default case of not providing arguments (e.g. I want the tox file to be convenient both locally and on CI) |
|
(FYI: I'm also using this PR to test the redirect mess as it's still not 100% up and working) |
52287fb to
3565257
Compare
|
OK, finally the redirector action works. I'm still puzzled why the context conditional didn't work, but leave that rabbit hole for another day. |
|
Cool. Testing this locally, I do see it successfully detecting local changes and only testing the changed notebooks. We just need to pull apart |
There are some problems with this, e.g. I don;t have
originlocally, and the logic didn't fully work with github URLs (nor I want to hardwire the repo link in the test file).So, do we want this to just be done in CI, but locally we test with all the files? I'm not fully certain what's the best approach.