sigma / magit-gh-pulls

Magit plugin for dealing with GitHub pull requests
254 stars 48 forks source link

too slow #14

Open cedricpinson opened 10 years ago

cedricpinson commented 10 years ago

Not sure it's relevant for everyone, but it's not useable on my use case. I wait about 15secs when I refresh the magit-status buffer. Maybe that would be good to cache results.

Can I help other than complaining ? like giving more informations ?

alexander-yakushev commented 10 years ago

The results should be cached. They usually aren't if magit-gh-pulls fails to parse remote URL properly. Please write what M-: (magit-gh-pulls-guess-repo) tells you.

dgtized commented 10 years ago

I'm also finding gh-pulls to be too slow to actually use on some repositories, notably private ones. When I ran magit-gh-pulls-guess-repo, it spits out the correct ("org" . "repo") dotted pair. I'm not using the latest magit though, still using the last stable release, not sure if that is an issue.

Just as a thought, perhaps it should operate similar to the branch manager as a PR manager instead. That way it won't block the main github UI refresh.

alexander-yakushev commented 10 years ago

Just as a thought, perhaps it should operate similar to the branch manager as a PR manager instead. That way it won't block the main github UI refresh.

You mean only showing pull request when a special PR manager mode is entered? This is possible, perhaps it can be configurable behaviour.

dgtized commented 10 years ago

Sorry, by github UI refresh I meant magit UI refresh. Allowing quick review access to PR's is excellent, but it's not clear that it needs to be in the default magit refresh screen, which is focused on the current branch status in relation to origin. Executing a remote request (cached or not) on each refresh of magit status seems rather heavy weight. Adding another manager ala branches focused on pull requests would constrain that cost to when a user is actively focused on managing PRs.

alexander-yakushev commented 10 years ago

Executing a remote request (cached or not) on each refresh of magit status seems rather heavy weight.

Well, that actually shouldn't happen. But I don't actually understand well how the caching works, so it's kind of magic to me. Can you please verify that you are using the correct versions of cache libraries required by magit-gh-pulls? Or do you mean that even cached requests take long to be rendered? Is it because there are a lot of pull requests there?

dgtized commented 10 years ago

I am using melpa, but:

pcache is

;; Version: 20131201.1159
;; X-Original-Version: 0.2.4

gh.el is

(define-package "gh" "20140706.1506" "A GitHub library for Emacs" '((eieio "1.3") (pcache "0.2.4") (logito "0.1")))

magit is:

(define-package "magit" "20140825.940" "control Git from Emacs" '((cl-lib "0.3") (git-commit-mode "0.14.0") (git-rebase-mode "0.14.0")) :keywords '("vc" "tools"))

emacs is: GNU Emacs 24.4.50.2 (x86_64-pc-linux-gnu, GTK+ Version 3.8.6) of 2014-06-08 on iridium

and magit-gh-pulls is:

(define-package "magit-gh-pulls" "20140829.629" "GitHub pull requests extension for Magit" '((emacs "24") (gh "0.4.3") (magit "1.1.0") (pcache "0.2.3") (s "1.6.1")) :url "https://github.com/sigma/magit-gh-pulls" :keywords '("tools"))

Anything else you need the version info for? Again, testing on a package like cider the PR menu is pretty snappy, but for my clone of projectile, it has a 45s initial request to github. Which is probably not going to disappear if it were in another menu, but at least would not interrupt calling magit-status. On private repos though, it never appears to cache the results so it blocks for 10s at least everytime I hit g to refresh magit-status. On another private repo with no active PR's it's snappy.

alexander-yakushev commented 10 years ago

Can you please confirm that in all cases (magit-gh-pulls-guess-repo) returns a fine pair of username and repository?

alexander-yakushev commented 10 years ago

I don't have private repositories on Github, so I can't really check why the API works so slow for them. I think I can only hope for someone other to take a look at why exactly the results aren't cached for them.

dgtized commented 10 years ago

All the cases that are slow return a valid pair. I do note that for my clone of cider it returns ("clojure-emacs". "cider") and not my fork. Also interesting, in the case of my own personal project github-clone.el (which is the root repo), it returns nil, but I don't currently have any open PR's so I can't really test the fetch speed there.

alexander-yakushev commented 9 years ago

@cedricpinson @dgtized I've just pushed a commit that disables automatic Github querying by default. Although it will only be available with 2.1.0 version of Magit that is to be released on July 1st.

robbyoconnor commented 9 years ago

Holy crap I hate to be that guy but me too...

alexander-yakushev commented 9 years ago

First of all, it shouldn't be slow now when you open the magit-buffer, only when you manually refresh the PR list. Then, which repository you try it with? Does it have many PRs? What's the output of M-: (magit-gh-pulls-guess-repo)?

alexander-yakushev commented 9 years ago

@cedricpinson @robbyoconnor I know it's been a while, but I think I have found what was causing the slowdown. I stumbled upon it when I started using private repositories myself. I hope the fix I provided will work for you in case you still use magit-gh-pulls.

robbyoconnor commented 9 years ago

I think it's fine now -- I'm not sure -- I guess I don't use it much -- it also seems to fail for some reason when I try to create a PR from magit-gh-pulls...not sure if that's a bug or not -- it could very well be PEBKAC

alexander-yakushev commented 9 years ago

In fact, if you still use magit-gh-pulls from MELPA then the latest version hasn't yet hit the repository. So wrt creating pull requests, I'd ask you to check for updates some time later if that doesn't distract you too much. Or have you grabbed the latest version from git?

robbyoconnor commented 9 years ago

It responded with HTTP code 422 for me :(

alexander-yakushev commented 9 years ago

That's weird. It returns 422 when it should have asked to push the branch?

robbyoconnor commented 9 years ago

I had already pushed my branch and got the 422 -- I wonder if it has to do with having contribution guidelines on the upstream repo?