-
Notifications
You must be signed in to change notification settings - Fork 58
We do actually want CI to pass a branch #1123
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
Conversation
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 sensible, but we should document it - https://github.com/matrix-org/sytest/blob/7178c8bd53557c739e99771bf67c9116ec0d731f/docker/README.md lists all the other env vars that the docker image understands, so we should include this one
docker/README.md
Outdated
homeserver git repository to be mounted at `/src`; otherwise, the image will | ||
try to fetch that repository from GitHub. |
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.
Is this correct? I thought that it would just fail if /src
didn't exist.
docker/README.md
Outdated
* `SYTEST_BRANCH`: controls which branch of synapse or dendrite we fetch when `/src` | ||
is missing. If omitted or if that branch doesn't exist, we'll fetch the appropriate development branch. |
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.
this isn't quite right. As above: if /src
is missing, I don't think it works at all...
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.
Argh! Thanks. I think I keep confusing /src
and /sytest
. It's the CI job that has to fetch the source
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.
No worries! It is pretty confusing.
I was confusing /sytest with /src, and bootstrap.sh with CI config.
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.
lgtm otherwise, thanks!
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
I'll run the docker |
PR #1123 broke the `Trying to get same-named sytest branch` stage for Dendrite CI which is still in Buildkite.
this pr not from my fork but from the repo itself