Unable to merge, continually getting "magit-gh-pulls-merge-pull-request: Please fetch pull request commits first"

[dakrone]

From within magit, (and after a # g to fetch the latest PRs), If I do:

# f to fetch the PR

then # m to merge it, I continually get "magit-gh-pulls-merge-pull-request: Please fetch pull request commits first".

Is there a step missing?

[alexander-yakushev]

After you fetch a pull request, can you open it with TAB? Do you see the list of commits?

[dakrone]

@alexander-yakushev hitting TAB doesn't do anything at all

In the magit-process buffer, I can see that the # f fetch is working though:

  0 git … fetch git@github.com:dakrone/elasticsearch.git add-pretouch-jvm-flag
From github.com:dakrone/elasticsearch
 * branch            add-pretouch-jvm-flag -> FETCH_HEAD

[alexander-yakushev]

Do you have the latest magit-gh-pulls and gh, both from MELPA?

[dakrone]

Yep, I just updated this morning, using magit-20160302.322, magit-gh-pulls-20160225.1402, and gh-20160222.1811

[alexander-yakushev]

Yes, I confirm this is a bug. Recent performance improvements by @thieman introduced a bug here; if there are too many pull requests, then we don't display additional information about them. Additional information includes their commits and if they are available.

I've pushed a quick fix that enables extra information for top 10 pull requests. For the time being you can customize magit-gh-pulls-pull-detail-limit to something big (e.g. 200). Other performance hacks by Travis made this one not so important, so that even with big limit magit-gh-pulls works quite well (I've tested it on elasticsearch repo).

[thieman]

Interesting, potentially we should just back out https://github.com/sigma/magit-gh-pulls/pull/74

[alexander-yakushev]

@thieman We could, but there is still difference in performance with limit 10 and limit 200. I have something else in mind, but it requires a bit of time to implement.

[dakrone]

Okay, increasing magit-gh-pulls-pull-detail-limit to 200 does fix the problem for now, but does make magit-gh-pulls slower to work with. I was able to successfully fetch and merge a PR from magit.

[alexander-yakushev]

@dakrone If you update magit-gh-pulls now you should be able to change the limit back to 10-20. But then you will be able to merge only those top 20 PRs.

[dakrone]

Okay, I'm able to merge (I unfortunately can't decrease the limit since I need to be able to merge old PRs), so I'm going to close this. Thanks!

[alexander-yakushev]

Good to hear. I hope a more flexible and fast solution will be ready sometime soon.

[ibrahima]

We could, but there is still difference in performance with limit 10 and limit 200. I have something else in mind, but it requires a bit of time to implement.

@alexander-yakushev what did you have in mind? I'd love it if #74 was backed out; it seems like just allowing the possibility of fetching more than 10 PRs shouldn't adversely affect performance, especially after #75 was merged. Been playing around a lot with this library, magit, and gh.el lately so I might be able to contribute if I knew what you were looking for. Thanks for maintaining this!

[braham-snyder]

Thanks for magit-gh-pulls!

Increasing magit-gh-pulls-pull-detail-limit worked for me as well -- perhaps this issue could be reopened so that this solution is easier to find?