6 ms·

I feel that Gitlab needs to optimize for the 90% use case instead of adding more features. For example does anyone else find Gitlabs diff lacking? It makes rev

I feel that Gitlab needs to optimize for the 90% use case instead of adding more features. For example does anyone else find Gitlabs diff lacking? It makes reviewing large patches painful, with the seemingly constant fetching of individual file diffs and inability to show large file diffs. The UI in general feels sluggish, especially when compared to something like Gerrit or GitHub.

ulrikrasmussen 6y ago
So much second this. It actually feels like it is getting slower on each upgrade, and more buggy. Am I the only one who finds that the "go to next unresolved" button now often goes into an inconsistent non-working state that requires a (slow) refresh? Given that I spend most of my time on GitLab reviewing, this is a real pain point for me.
- phikai 6y ago
  > Am I the only one who finds that the "go to next unresolved" button now often goes into an inconsistent non-working state that requires a (slow) refresh? I don't think I've seen that reported before and we have been working through some interesting to reproduce issues in recent milestones. Would you mind opening an issue for this? Feel free to tag me (same name).
  - acdha 6y ago
    There's an open issue for one thing I notice all the time: the next/previous unresolved commands update some internal state but scrolling and other navigation do not. This means that if you use any other navigation mechanism (e.g. search) the next time you hit “go to next unresolved” button it will instead scroll backwards to whatever thread follows the last one you navigated to using the “go to next unresolved” mechanism even if that's scrolling hundreds of lines back.
  - shubhamaggarwal 6y ago
    There’s already an issue for this: https://gitlab.com/gitlab-org/gitlab/-/issues/281947 https://gitlab.com/gitlab-org/gitlab/-/issues/281947
    - ulrikrasmussen 6y ago
      Yes. This is it. Thanks! I wasn't actually sure when it broke, but this issue describes it perfectly.
- adobeslabs 6y ago
  every day I wish I could just drop a comment in the webIDE for an MR.. https://gitlab.com/gitlab-org/gitlab-foss/-/issues/48790 https://gitlab.com/gitlab-org/gitlab-foss/-/issues/48790
diegocg 6y ago
The thing I miss the most from the reviewing process is the ability to review diffs per-commit. This is closer to how git is used in the linux mailing lists but gitlab completely breaks that workflow. Github has added some support for that. And yeah, performance sucks, even for trivial patches.
- john_cogs 6y ago
  When using merge requests, each push to a branch creates a new version. You can then compare between any two versions and review the diff. Documentation: https://docs.gitlab.com/ee/user/project/merge_requests/versions.html https://docs.gitlab.com/ee/user/project/merge_requests/versi...
  - diegocg 6y ago
    Not very helpful unless you push every commit individually
- phikai 6y ago
  Hi! I'm not sure if you've seen this, but in GitLab 13.0 we released commit based navigation for merge requests. You can read more about this in our docs, but we'd love any feedback you have on the feature. Documentation: https://docs.gitlab.com/ee/user/project/merge_requests/reviewing_and_managing_merge_requests.html#merge-requests-commit-navigation https://docs.gitlab.com/ee/user/project/merge_requests/revie...
acdha 6y ago
I'm really noticing the performance on moderate-size code reviews — once you're over a few hundred lines changing their Vue.js code devolves into a JavaScript benchmark doing whatever it does before updating the DOM.
- andr3 6y ago
  Hi, I'm André, the Frontend Eng Manager at GitLab for Code Review. Thanks for that comment. As you can expect, we notice it too. This is something the team is actively monitoring and working to improve (example: https://gitlab.com/gitlab-org/gitlab/-/issues/321691 https://gitlab.com/gitlab-org/gitlab/-/issues/321691 being worked on in the current milestone). I am curious: have you tried the file-by-file mode? Some users have given us very positive feedback on this feature, others not so much. It might be something that fits some users' habits but not others. But thought it was worth mentioning. We're actually considering and researching turning this on automatically for MRs over a certain size (https://gitlab.com/gitlab-org/gitlab/-/issues/212849 https://gitlab.com/gitlab-org/gitlab/-/issues/212849 your thoughts would be very appreciated there, thanks!). https://docs.gitlab.com/ee/user/project/merge_requests/reviewing_and_managing_merge_requests.html#file-by-file-diff-navigation https://docs.gitlab.com/ee/user/project/merge_requests/revie... Not that it solves the root of the problem (that's being addressed too) yet this feature allows for really large MRs to be reviewed without having to render all the changes at once.
phikai 6y ago
Hi! Thanks for the feedback on diffs - it's something we're always thinking about and working on in the group. One of the things we're currently working on is putting monitoring in pace for all the of the diff limits - https://gitlab.com/gitlab-org/gitlab/-/issues/31063 https://gitlab.com/gitlab-org/gitlab/-/issues/31063. The goal here is that we can further fine tune some of the limits in place to continue to help with larger merge requests. The group also spent some time investigating ways to improve the blocking time for large merge requests and you can see some of the discussion around that here: https://gitlab.com/gitlab-org/gitlab/-/issues/295237 https://gitlab.com/gitlab-org/gitlab/-/issues/295237. The last issue I'll drop here is https://gitlab.com/gitlab-org/gitlab/-/issues/241841 https://gitlab.com/gitlab-org/gitlab/-/issues/241841. You can also see all of the other issues related to performance that the Code Review group has been working on in GitLab here: https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&utf8=%E2%9C%93&state=closed&label_name[]=group%3A%3Acode%20review&label_name[]=performance https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&utf8... Thanks again for the feedback - and know that this is something we are looking in to.
phikai 6y ago
One more thought on all of this - we're currently doing some research on problems users face in the merge request. Feel free to take our survey: https://gitlab.fra1.qualtrics.com/jfe/form/SV_9HQQil77CjHJICW https://gitlab.fra1.qualtrics.com/jfe/form/SV_9HQQil77CjHJIC... Bonus... you can enter for a chance to win a $75 Amazon gift card.
- apple4ever 6y ago
  REBASE IS HORRIBLE! That's a good first step to fix. (And yes I filled out the survey).
- malinens 6y ago
  Docker runner performance is horrible. By using ssh runner which runs exactly the same docker command I save 7 seconds each time. Thid leeds to many hacks to get performance needed (jobs are merged in one big one for example). With many smaller jobs some pipelines run sometimes up to 50% slower than ssh + docker. Also built-in caching is painfully slow. Our self hosted minio instance as custom cache is 10 times faster than Gitlab's local cache and this is absurd. These are 2 main speed issues with me. As code reviewer large diff bad performance would be the 3rd issue. It lags even with my beefy workstation. Maybe adding JS functionality is needed for files in current viewport? I don't need syntax highlight and comments options on all 100 files at the same time but I want an option to scroll down and this functionality on currently viewed files
  - deastman 6y ago
    Hi - can you create an issue in the gitlab-runner project? It would be great so that we can investigate further. https://gitlab.com/gitlab-org/gitlab-runner/-/issues https://gitlab.com/gitlab-org/gitlab-runner/-/issues
majkinetor 6y ago
Really looking forward for VSCode full blown merge reviewing. Impatiently wafting for this so I don't have to use sluggish browser any more: https://gitlab.com/gitlab-org/gitlab-vscode-extension/-/issues/342 https://gitlab.com/gitlab-org/gitlab-vscode-extension/-/issu...
- phikai 6y ago
  I'm so excited for this too! I'm really excited to continue bringing GitLab workflows closer to where developers are doing their work and hope this is going to be a really meaningful step in that direction. I'm also interested to see what kind of impact it has on large MRs, but do know it's not the answer for what happens in GitLab.
apple4ever 6y ago
Agreed. Diffs and MRs in general are terrible. We get 500 errors constantly when submitting MRs, if we can push up at all. And don't try to rebase with the button - OH MY it will just fail. Or it will succeed and the interface will not update. Yuck. We are switching to GitHub. Which saddens me because I much prefer GitLab's interface. But I have lost the battle, due to GitLab's terrible performance.
scottweitzner 6y ago
I actually really like Gitlab’s, but that’s only because we moved from Gitlab to Azure Dev Ops...

ulrikrasmussen 6y ago

So much second this. It actually feels like it is getting slower on each upgrade, and more buggy. Am I the only one who finds that the "go to next unresolved" button now often goes into an inconsistent non-working state that requires a (slow) refresh? Given that I spend most of my time on GitLab reviewing, this is a real pain point for me.

phikai 6y ago
> Am I the only one who finds that the "go to next unresolved" button now often goes into an inconsistent non-working state that requires a (slow) refresh? I don't think I've seen that reported before and we have been working through some interesting to reproduce issues in recent milestones. Would you mind opening an issue for this? Feel free to tag me (same name).
- acdha 6y ago
  There's an open issue for one thing I notice all the time: the next/previous unresolved commands update some internal state but scrolling and other navigation do not. This means that if you use any other navigation mechanism (e.g. search) the next time you hit “go to next unresolved” button it will instead scroll backwards to whatever thread follows the last one you navigated to using the “go to next unresolved” mechanism even if that's scrolling hundreds of lines back.
- shubhamaggarwal 6y ago
  There’s already an issue for this: https://gitlab.com/gitlab-org/gitlab/-/issues/281947 https://gitlab.com/gitlab-org/gitlab/-/issues/281947
  - ulrikrasmussen 6y ago
    Yes. This is it. Thanks! I wasn't actually sure when it broke, but this issue describes it perfectly.
adobeslabs 6y ago
every day I wish I could just drop a comment in the webIDE for an MR.. https://gitlab.com/gitlab-org/gitlab-foss/-/issues/48790 https://gitlab.com/gitlab-org/gitlab-foss/-/issues/48790

diegocg 6y ago

The thing I miss the most from the reviewing process is the ability to review diffs per-commit. This is closer to how git is used in the linux mailing lists but gitlab completely breaks that workflow. Github has added some support for that. And yeah, performance sucks, even for trivial patches.

john_cogs 6y ago
When using merge requests, each push to a branch creates a new version. You can then compare between any two versions and review the diff. Documentation: https://docs.gitlab.com/ee/user/project/merge_requests/versions.html https://docs.gitlab.com/ee/user/project/merge_requests/versi...
- diegocg 6y ago
  Not very helpful unless you push every commit individually
phikai 6y ago
Hi! I'm not sure if you've seen this, but in GitLab 13.0 we released commit based navigation for merge requests. You can read more about this in our docs, but we'd love any feedback you have on the feature. Documentation: https://docs.gitlab.com/ee/user/project/merge_requests/reviewing_and_managing_merge_requests.html#merge-requests-commit-navigation https://docs.gitlab.com/ee/user/project/merge_requests/revie...

acdha 6y ago

I'm really noticing the performance on moderate-size code reviews — once you're over a few hundred lines changing their Vue.js code devolves into a JavaScript benchmark doing whatever it does before updating the DOM.

andr3 6y ago
Hi, I'm André, the Frontend Eng Manager at GitLab for Code Review. Thanks for that comment. As you can expect, we notice it too. This is something the team is actively monitoring and working to improve (example: https://gitlab.com/gitlab-org/gitlab/-/issues/321691 https://gitlab.com/gitlab-org/gitlab/-/issues/321691 being worked on in the current milestone). I am curious: have you tried the file-by-file mode? Some users have given us very positive feedback on this feature, others not so much. It might be something that fits some users' habits but not others. But thought it was worth mentioning. We're actually considering and researching turning this on automatically for MRs over a certain size (https://gitlab.com/gitlab-org/gitlab/-/issues/212849 https://gitlab.com/gitlab-org/gitlab/-/issues/212849 your thoughts would be very appreciated there, thanks!). https://docs.gitlab.com/ee/user/project/merge_requests/reviewing_and_managing_merge_requests.html#file-by-file-diff-navigation https://docs.gitlab.com/ee/user/project/merge_requests/revie... Not that it solves the root of the problem (that's being addressed too) yet this feature allows for really large MRs to be reviewed without having to render all the changes at once.

phikai 6y ago

Hi! Thanks for the feedback on diffs - it's something we're always thinking about and working on in the group. One of the things we're currently working on is putting monitoring in pace for all the of the diff limits - https://gitlab.com/gitlab-org/gitlab/-/issues/31063 https://gitlab.com/gitlab-org/gitlab/-/issues/31063. The goal here is that we can further fine tune some of the limits in place to continue to help with larger merge requests. The group also spent some time investigating ways to improve the blocking time for large merge requests and you can see some of the discussion around that here: https://gitlab.com/gitlab-org/gitlab/-/issues/295237 https://gitlab.com/gitlab-org/gitlab/-/issues/295237. The last issue I'll drop here is https://gitlab.com/gitlab-org/gitlab/-/issues/241841 https://gitlab.com/gitlab-org/gitlab/-/issues/241841. You can also see all of the other issues related to performance that the Code Review group has been working on in GitLab here: https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&utf8=%E2%9C%93&state=closed&label_name[]=group%3A%3Acode%20review&label_name[]=performance https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&utf8... Thanks again for the feedback - and know that this is something we are looking in to.

phikai 6y ago

One more thought on all of this - we're currently doing some research on problems users face in the merge request. Feel free to take our survey: https://gitlab.fra1.qualtrics.com/jfe/form/SV_9HQQil77CjHJICW https://gitlab.fra1.qualtrics.com/jfe/form/SV_9HQQil77CjHJIC... Bonus... you can enter for a chance to win a $75 Amazon gift card.

apple4ever 6y ago
REBASE IS HORRIBLE! That's a good first step to fix. (And yes I filled out the survey).
malinens 6y ago
Docker runner performance is horrible. By using ssh runner which runs exactly the same docker command I save 7 seconds each time. Thid leeds to many hacks to get performance needed (jobs are merged in one big one for example). With many smaller jobs some pipelines run sometimes up to 50% slower than ssh + docker. Also built-in caching is painfully slow. Our self hosted minio instance as custom cache is 10 times faster than Gitlab's local cache and this is absurd. These are 2 main speed issues with me. As code reviewer large diff bad performance would be the 3rd issue. It lags even with my beefy workstation. Maybe adding JS functionality is needed for files in current viewport? I don't need syntax highlight and comments options on all 100 files at the same time but I want an option to scroll down and this functionality on currently viewed files
- deastman 6y ago
  Hi - can you create an issue in the gitlab-runner project? It would be great so that we can investigate further. https://gitlab.com/gitlab-org/gitlab-runner/-/issues https://gitlab.com/gitlab-org/gitlab-runner/-/issues

majkinetor 6y ago

Really looking forward for VSCode full blown merge reviewing. Impatiently wafting for this so I don't have to use sluggish browser any more: https://gitlab.com/gitlab-org/gitlab-vscode-extension/-/issues/342 https://gitlab.com/gitlab-org/gitlab-vscode-extension/-/issu...

phikai 6y ago
I'm so excited for this too! I'm really excited to continue bringing GitLab workflows closer to where developers are doing their work and hope this is going to be a really meaningful step in that direction. I'm also interested to see what kind of impact it has on large MRs, but do know it's not the answer for what happens in GitLab.

apple4ever 6y ago

Agreed. Diffs and MRs in general are terrible. We get 500 errors constantly when submitting MRs, if we can push up at all. And don't try to rebase with the button - OH MY it will just fail. Or it will succeed and the interface will not update. Yuck. We are switching to GitHub. Which saddens me because I much prefer GitLab's interface. But I have lost the battle, due to GitLab's terrible performance.

scottweitzner 6y ago

I actually really like Gitlab’s, but that’s only because we moved from Gitlab to Azure Dev Ops...