softdevteam / ykrustc

Yorick Meta-tracer
Other
6 stars 4 forks source link

Patch the yk dep using a special line in the PR description. #146

Closed vext01 closed 3 years ago

vext01 commented 3 years ago

I got this working towards the end of last week. I think we can start the review now.

When doing a ykrustc pull request (PR), if the PR description contains a lineci-yk: <github-user> <branch> then the Cargo.toml file is (temporarily) patched to override the dependency.

Once a ykrustc PR is merged, the yk branch should be merged immediately after. We can't do that automatically unfortunately.

Testing

I'll test this now by overriding the yk dependency to a branch I've deliberately broken.

ci-yk: vext01 broken-branch

vext01 commented 3 years ago

bors try

ltratt commented 3 years ago

Apart from that one comment, this looks good to me!

vext01 commented 3 years ago

bors try-

vext01 commented 3 years ago

bors try

vext01 commented 3 years ago

My apologies. The line should be ci-yk not yk-ci. I can see from the CI log that it didn't patch.

bors try-

vext01 commented 3 years ago

Fixed description. Let's try again.

bors try

vext01 commented 3 years ago

There we go:

+ /opt/buildbot/bin/python3 .buildbot_patch_yk_dep.py
Patching yk dependency to: https://github.com/vext01/yk broken-branch
bors[bot] commented 3 years ago

try

Build failed:

vext01 commented 3 years ago
error: expected one of `(`, `[`, or `{`, found keyword `pub`
  --> /home/buildbot-worker3/.cargo/git/checkouts/yk-d2e2b9740fea6468/f581721/ykpack/src/types.rs:13:1
   |
10 | CARNAGE!
   |         - expected one of `(`, `[`, or `{`

Good!

Now, I haven't tested r+ and the commit message format is slightly different. Let's leave the override in and attempt a merge. It should fail.

Please do a r+.

ltratt commented 3 years ago

bors r+

bors[bot] commented 3 years ago

Build failed:

vext01 commented 3 years ago

Good. Now I've removed the line, if you r+ again, it should merge ok.

ltratt commented 3 years ago

bors r+

vext01 commented 3 years ago

Looking good. It doesn't appear to have patched any deps.

bors[bot] commented 3 years ago

Build succeeded: