@danmosemsft commented on Tue Aug 29 2017
Maestro seems to force push to its version update PR branch, if the branch has not been merged yet. The problem with this is that the history of test failures on that PR is lost, or at least very hard to recover.
For example in dotnet/corefx#23563 there are a bunch of test failures that are taking time to unravel. Because of the force pushes, I can't look at the PR and tell which version introduced the test failure. It may have been the first couple of versions of the PR worked perfectly (just nobody hit merge yet) and then the tests started failing. All I see is 20 odd title changes.
Can Maestro just push a new commit, to help avoid this?
@danmosemsft commented on Tue Aug 29 2017
cc @stephentoub
@dagood commented on Thu Aug 31 2017
I don't think there's a blocker, but there is significant flow rework in BuildTools/VersionTools to do it. The big reason force push works best for this right now is that it's very simple: the auto-update definition checks out the target branch, e.g. master, and all update has to do is make a new commit and push it to the branch.
I think the best way to do this is to teach auto-update to find the PR before making changes, fetch the branch, and check out the latest PR commit before performing the update. We have code that finds the PR to avoid overwriting user commits, so we should at least have the raw API calls.
@danmosemsft commented on Thu Aug 31 2017
If you work on this code, we should consider whether it can help show what changes it is bringing. Right now I have to do this manually (and most people won't know how):
https://github.com/dotnet/corefx/issues/23586#issuecomment-325861191
This is super useful when root causing CI failures with these jobs.
@danmosemsft commented on Thu Aug 31 2017
Another drawback of force push is that it will blow away any build break fixes I push to the branch.
@dagood commented on Thu Aug 31 2017
The version update code actually posts a comment instead of pushing if you've made any fixes on the branch, for example: dotnet/corefx#22132 (comment). (That's the "We have code that finds the PR to avoid overwriting user commits,")
To link it in, showing commits is tracked by #1141. Currently there's no way for the update tooling to do the job any better than your process--in theory it could be automated that way, but there is an upcoming spec for a new format we can use on dotnet/versions to transfer the extra info.
@danmosemsft commented on Tue Aug 29 2017
Maestro seems to force push to its version update PR branch, if the branch has not been merged yet. The problem with this is that the history of test failures on that PR is lost, or at least very hard to recover.
For example in dotnet/corefx#23563 there are a bunch of test failures that are taking time to unravel. Because of the force pushes, I can't look at the PR and tell which version introduced the test failure. It may have been the first couple of versions of the PR worked perfectly (just nobody hit merge yet) and then the tests started failing. All I see is 20 odd title changes.
Can Maestro just push a new commit, to help avoid this?
@danmosemsft commented on Tue Aug 29 2017
cc @stephentoub
@dagood commented on Thu Aug 31 2017
I don't think there's a blocker, but there is significant flow rework in BuildTools/VersionTools to do it. The big reason force push works best for this right now is that it's very simple: the auto-update definition checks out the target branch, e.g.
master, and all update has to do is make a new commit and push it to the branch.I think the best way to do this is to teach auto-update to find the PR before making changes, fetch the branch, and check out the latest PR commit before performing the update. We have code that finds the PR to avoid overwriting user commits, so we should at least have the raw API calls.
@danmosemsft commented on Thu Aug 31 2017
If you work on this code, we should consider whether it can help show what changes it is bringing. Right now I have to do this manually (and most people won't know how):
https://github.com/dotnet/corefx/issues/23586#issuecomment-325861191
This is super useful when root causing CI failures with these jobs.
@danmosemsft commented on Thu Aug 31 2017
Another drawback of force push is that it will blow away any build break fixes I push to the branch.
@dagood commented on Thu Aug 31 2017
The version update code actually posts a comment instead of pushing if you've made any fixes on the branch, for example: dotnet/corefx#22132 (comment). (That's the "We have code that finds the PR to avoid overwriting user commits,")
To link it in, showing commits is tracked by #1141. Currently there's no way for the update tooling to do the job any better than your process--in theory it could be automated that way, but there is an upcoming spec for a new format we can use on dotnet/versions to transfer the extra info.