Skip to content
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

PRs can sometimes take a long time to merge #293

Closed
amoffat opened this issue May 26, 2017 · 4 comments
Closed

PRs can sometimes take a long time to merge #293

amoffat opened this issue May 26, 2017 · 4 comments

Comments

@amoffat
Copy link
Contributor

amoffat commented May 26, 2017

Something I noticed with the following two pull requests, which were opened 8 and 10 hours ago respectively #280 #278, but still hadn't merged. Both PRs have a single commit, but Chaos was saying their voting window ended at the same time, despite the PRs being opened hours apart.

Digging into the merge_commit_sha 630d9e5 (which we use for "reliable" branch push times) for #278:

 'commit': {'author': {'date': '2017-05-26T17:28:44Z',
                       'email': 'philipp@rukin.me',
                       'name': 'Phil Rukin'},
            'comment_count': 0,
            'committer': {'date': '2017-05-26T17:28:44Z',
                          'email': 'noreply@github.com',
                          'name': 'GitHub'},
            'message': 'Merge ae12b7022a2b81fca5e0b4bed22baaf4b9c55aa0 into '
                       'd1cfad21414e019975c4a316faad20029b9afe07',
            'tree': {'sha': '46ad01b78c0194e5a8399cd5a7b8c13d76853cc5',
                     'url': 'https://api.github.com/repos/chaosbot/Chaos/git/trees/46ad01b78c0194e5a8399cd5a7b8c13d76853cc5'},
            'url': 'https://api.github.com/repos/chaosbot/Chaos/git/commits/630d9e50a30d6bbcc1ca7fa4639628b88d5231ee'},

And comparing to #278's single commit ae12b70:

  'commit': {'author': {'date': '2017-05-26T10:07:13Z',
                        'email': 'philipp@rukin.me',
                        'name': 'Phil Rukin'},
             'comment_count': 0,
             'committer': {'date': '2017-05-26T10:07:13Z',
                           'email': 'philipp@rukin.me',
                           'name': 'Phil Rukin'},
             'message': 'Add homepage to the repo\n'
                        'See github api docs here: '
                        'https://developer.github.com/v3/repos/#edit',
             'tree': {'sha': 'e68d1cb57377f02e69f8ae9bf16501a4b1f506c1',
                      'url': 'https://api.github.com/repos/chaosbot/Chaos/git/trees/e68d1cb57377f02e69f8ae9bf16501a4b1f506c1'},

It looks like the Github may sometimes have a significant delay between a push to a PR and the merge_commit_sha existing. In this case, it was over 7 hours(!) I did the same comparison for #280, and the merge_commit_sha f0fd79b has the exact same timestamp as the one from #278, 630d9e5. This would suggest that maybe Github's merge commit job runner had died, restarted hours later, and crunched through the pending merge test jobs.

So maybe this isn't something to worry about yet, but it may be important to keep in mind if people wonder why their PRs are taking so long

@amoffat
Copy link
Contributor Author

amoffat commented May 26, 2017

In the time it took me to write that, the merge commit for #278 got reset again 6a8e6b4 and pushed the voting window back, for no apparent reason:

 'commit': {'author': {'date': '2017-05-26T20:28:45Z',
                       'email': 'philipp@rukin.me',
                       'name': 'Phil Rukin'},
            'comment_count': 0,
            'committer': {'date': '2017-05-26T20:28:45Z',
                          'email': 'noreply@github.com',
                          'name': 'GitHub'},
            'message': 'Merge ae12b7022a2b81fca5e0b4bed22baaf4b9c55aa0 into '
                       '4cae365b4094bc9ce73635a46bcb4cfbe58d1e0e',
            'tree': {'sha': '7f9081fb6075e21c7ccf1cbb1ea48c4edee72dcf',
                     'url': 'https://api.github.com/repos/chaosbot/Chaos/git/trees/7f9081fb6075e21c7ccf1cbb1ea48c4edee72dcf'},
            'url': 'https://api.github.com/repos/chaosbot/Chaos/git/commits/6a8e6b4e20d023e183f15ba18dc22ab7fca0e99d'},

I think the merge_commit_sha stuff might need to be reverted? Wish I had a github contact :)

@phil-r
Copy link
Member

phil-r commented May 26, 2017

@amoffat Tell me If I can help in any way ^_^
P.S. I actually wanted to ask why this is happening, but I've seen some PRs related to change of voting window and thought it's related.

P.P.S Is it possible that it's because I'm in CET(GMT+2 right now)?

@amoffat
Copy link
Contributor Author

amoffat commented May 26, 2017

@phil-r thank you! I think you are fine, this is probably an issue with github :) I've reached out to them to see if they know what's going on. If I don't hear from them soon, I'll revert the changes that introduced this problem. Then your stuff should merge in a more timely manner

@amoffat
Copy link
Contributor Author

amoffat commented May 26, 2017

I know why the temp merge_commit_sha keeps resetting... it has to reset each time master gets a push, not just the branch backing the PR. So what was happening was, one PR would get merged in, causing all PRs to reset their vote window. Then another PR would get merged in, etc. Bad. Reverting #271

@amoffat amoffat closed this as completed May 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants