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

Do not fetch all refs in pull-request compare #7797

Merged
merged 2 commits into from Aug 13, 2019

Conversation

@typeless
Copy link
Contributor

commented Aug 8, 2019

Fixes #2837 again

Do not fetch all refs
Which would unnecessarily slow down the pull compare operation.
@codecov-io

This comment has been minimized.

Copy link

commented Aug 8, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@c662f4c). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #7797   +/-   ##
=========================================
  Coverage          ?   41.39%           
=========================================
  Files             ?      474           
  Lines             ?    63780           
  Branches          ?        0           
=========================================
  Hits              ?    26400           
  Misses            ?    33942           
  Partials          ?     3438
Impacted Files Coverage Δ
modules/git/repo_compare.go 63.63% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c662f4c...5e5a051. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 label Aug 8, 2019

@lafriks
lafriks approved these changes Aug 8, 2019

@GiteaBot GiteaBot added lgtm/need 1 and removed lgtm/need 2 labels Aug 8, 2019

@lafriks lafriks added the kind/bug label Aug 8, 2019

@lafriks lafriks added this to the 1.10.0 milestone Aug 8, 2019

@lunny

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

@typeless Have you tried this?

@1902sysad

This comment has been minimized.

Copy link

commented Aug 9, 2019

will this be merge in 1.9.0.1?

@typeless

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

@lunny I just ran the integration test for it. I can test it at my workplace next Mondy, where I have access to a huge repo.

@1902sysad does the pr work for you?

@typeless

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

I have confirmed that git remote -f is no longer the bottleneck when doing new pull request. The next bottleneck seems to be git diff -M according to my ps -u git -o cmd sampling.

@lunny
lunny approved these changes Aug 13, 2019

@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 1 labels Aug 13, 2019

@lafriks lafriks merged commit 602155d into go-gitea:master Aug 13, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr Build is passing
Details
@lafriks

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

please send backport

@1902sysad

This comment has been minimized.

Copy link

commented Aug 16, 2019

@typeless i have tested this to a big repository it works fine! :D

when this will be a part of stable release? :)

Thank you.

@lunny

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

@1902sysad 1.9.1 has released which included the backport #7837

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.