PR internally edited as CL are not automatically merged

[bhack]

Community PRs that are internally edited by TF members on the mirrored CL are not going to be automatically merge the PR and they needs to be manually closed.
Other then requiring extra manual work it is going to alter some automated stats as these will be classified as community rejected PRs instead that accepted PRs.

This is the list limited to 2022:
PR: #55413 Closed at: 2022-04-07 11:27:48
PR: #55469 Closed at: 2022-04-07 11:22:12
PR: #54223 Closed at: 2022-04-01 14:28:30
PR: #55317 Closed at: 2022-04-04 20:05:50
PR: #55026 Closed at: 2022-03-14 14:15:00
PR: #54860 Closed at: 2022-03-14 14:11:03
PR: #53437 Closed at: 2022-03-23 20:52:56
PR: #53367 Closed at: 2022-02-23 16:37:52
PR: #54426 Closed at: 2022-02-18 17:38:06
PR: #53507 Closed at: 2022-02-02 16:21:27
PR: #53536 Closed at: 2022-01-13 12:30:37
PR: #50834 Closed at: 2022-01-10 12:23:31

Some solution to explore:

• Can we push to the PR branch head before merge with Copybara pr_branch_to_update?
• Or can the internal team push directly to the pr_branch as the other community members as most of the pr_branches are writable aslo by internal codeowners?

[bhack]

/cc @theadactyl @yarri-oss

[mihaimaruseac]

This seems fixed now as of tensorflow/tensorflow#54377

[bhack]

We are not in the same case as the listed PRs.

Your example was always working as the merge commit you have pushed in master has the right reference parent on the PR branch head hash.

I don't know what they are doing internally on the PRs I've listed above but the merge commit pushed on master don't reference anymore the PR branch head as a partent.

So there is still something in the internal development practice that break the PR branch reference.

[bhack]

As you are example is a regular I think it is not related to the problem we have.
We need to take the first PR in my list:
https://github.com/tensorflow/tensorflow/pull/55413/commits

This commit (PR branch head) is not available in master as the merge pushed in master has no reference of this commit in its parents (It has it instead regularly in your case). So it is impossible for Github to understand that you are merging the commit of the branch and close it with a merged status:

7c0b775 parent in the push merge is not the branch head but an intermediate commit.

Please can you can internally check, as I have no visibility, what happened in PiperOrigin-RevId: 439970863 and the related CL?

As your example is linear and regular It is important to understand what they are doing to remove any reference to the branch head commit in the merge.

Edit:
Is it possible that something is not working in importing new commits from the PR branch after it was imported in the CL the first time? It seems that the CL was blocked on Mar, 31 2022 commit instead of the Apr, 1 2022 head.

[bhack]

It happened again now on tensorflow/tensorflow#55529:

PiperOrigin-RevId: 442524189

What difference you see internally from tensorflow/tensorflow#54377 (PiperOrigin-RevId: 442056945)?

[bhack]

5 hours ago, another pushed merge on an intermediate PR commit:
tensorflow/tensorflow#55518

[bhack]

Can we reopen this to track why the CL merge sometimes with an intermediate PR branch commit?

I am tracking instead, with the Github support, the case where the merge commit has the right parent pointing on the PR branch head sha.

[mihaimaruseac]

This is a different scenario than the one we considered when this was closed.

[bhack]

I think here we could focus on the first point:

why the CL merge sometimes with an intermediate PR branch commit

[mihaimaruseac]

So, for tensorflow/tensorflow#55529, the internal changelist looks like

This diff is not different than the PR

Also, the merge commit matches the last commit on the branch:

Is there something I'm misunderstanding from #413 (comment) ?

The internal changelist has last been approved on Apr 18th, when the PR got merged, but the last PR commit has been from Apr 13th.

The only strange thing I see is that the PR has a merge from master back into it, instead of a rebase. This causes the PR merge message to not be complete?

[mihaimaruseac]

Though the merge theory doesn't apply for tensorflow/tensorflow#55518

[mihaimaruseac]

Updated: this seems to be a internal tooling problem. Internal team has been notified

[bhack]

Updated: this seems to be a internal tooling problem. Internal team has been notified

Just to add another case on the TF/infra side:
tensorflow/tensorflow#48056
tensorflow/tensorflow@299cb76

The PR is still open.

[bhack]

Another case closed just with the Github pattern (not-merged) tensorflow/tensorflow#53905

[bhack]

Updated: this seems to be a internal tooling problem. Internal team has been notified

@mihaimaruseac Can you share the internal related tickets with @theadactyl?

Thanks

[bhack]

/cc @MichaelHudgins