Travis: auto rebase on master before running #1526
Conversation
|
Makes sense, Travis is unhappy though. |
| @@ -22,7 +22,12 @@ install: | |||
| - git config --global user.email "travis@example.com" | |||
| - git config --global user.name "Travis CI" | |||
|
|
|||
| - git remote add riot https://github.com/RIOT-OS/RIOT.git | |||
| - git fetch riot master --depth=1 | |||
There was a problem hiding this comment.
The --depth=1 (as far as I always understood it) option could lead to [edit: a higher risk of] merge errors if the merge base is not present.
There was a problem hiding this comment.
Joa, I'll just delete the parameter.
There was a problem hiding this comment.
(just to emphasis how dangerous this is: after testing this my repository now isn't working anymore, because there is now this 4546c3f commit in it, that has no parents and basically leads to me not being able to do anything sensible)
|
restarting Travis |
|
It works. |
| script: | ||
| - git rebase riot/master || true |
There was a problem hiding this comment.
Wouldn't it make sense to git rebase --abort if the rebase failed?
There was a problem hiding this comment.
Oops, totally right.
|
ACK |
|
Btw. this works, too: https://travis-ci.org/RIOT-OS/RIOT/builds/32233979#L1148 |
It can happen that two PRs don't interfere with each other in the sense that they hurt automatic rebasing, but still do not work together. The prime example is PR1 changes the API of some function that PR2 uses. If PR1 is merged, and PR2 is not rebased before merging, the error might get unnoticed before the next build. Travis CI would have have told both PR1 and PR2 are fine, but will complain (rather unnoticedly) that our master does not compile. This PR automatically rebases the PR on top of the current master, before running the tests. If the automatic rebase fails, then this is fine, because you will need to manually rebase again before merging, anyway. The manual rebase, i.e. new push, will trigger Travis CI. So, the main idea of this PR is that to can hit the "Restart Build" button in Travis CI before hitting the merge button in Github.
Travis: auto rebase on master before running
|
Nice @ retry |
|
Actually, I don't get the purpose of this PR. When a PR B won't work after a PR A got merged, Travis would fail anyway, wouldn't it? |
|
Only after you rebase. |
|
But why would you rebase? Usually a simple merge should do it. |
|
But if you merge in the master, then it's wrong order, isn't it? |
Why? Isn't that exactly what would happen if you hit the merge button? |
|
I second @OlegHahm. |
|
Why didn't you say anything in the ten days that the PR was open?.. |
|
Because I just realized that ;-) |
|
Sorry, I didn't understand the description but was trusting the review. Only realized now that some old PRs were failing due to this PR. |
|
Failing PRs were the idea of this PR. The question is if the other PRs are falsely failing. Can you name examples? |
|
I cannot recall which PR it was, but it was failing because of missing blacklisting for some applications on ATMega2560 that should have nothing to do with the PR. |
|
This behavior is the very intention of this PR:
QED |
|
I still don't see the reason for rebasing instead of merging. |
It can happen that two PRs don't interfere with each other in the sense
that they hurt automatic rebasing, but still do not work together. The
prime example is PR1 changes the API of some function that PR2 uses. If
PR1 is merged, and PR2 is not rebased before merging, the error might
get unnoticed before the next build. Travis CI would have have told both
PR1 and PR2 are fine, but will complain (rather unnoticedly) that our
master does not compile.
This PR automatically rebases the PR on top of the current master,
before running the tests. If the automatic rebase fails, then this is
fine, because you will need to manually rebase again before merging,
anyway. The manual rebase, i.e. new push, will trigger Travis CI.
So, the main idea of this PR is that to can hit the "Restart Build"
button in Travis CI before hitting the merge button in Github.