Hacker Newsnew | past | comments | ask | show | jobs | submit | andr3's commentslogin

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 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 your thoughts would be very appreciated there, thanks!). 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.


Thanks for the comment.

I've left a few replies in this thread already with more detail of work we have done, is currently in flight and work that is planned to keep improving the overall situation.

As to the "per commit" review, we're working on something to make this navigation easier and easier and shipping it very soon. https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28596

As to the collapsed, unfortunately if we loaded all diff lines expanded, the large changesets would make the browser unusable. We're working to lower the memory footprint and have better ways to deal with these large MRs.

One example I can leave you with that we're trying to figure out a way to make this useful as an additional option, is this: * File by file mode of reviewing MRs, including the concept of "unread diffs": https://gitlab.com/groups/gitlab-org/-/epics/516

We're aware of several of the pain points (we use GitLab to build GitLab ;) ), but if you have further feedback, please leave a comment in the issues and epics I've left here (or create new ones) and feel free to tag me @andr3.

Thanks!


Hi, Engineering Manager at GitLab here, Source Code team.

Thanks for the comment.

Indeed that is something we've had in our minds for a long time. Over the past months/years, we have been shipping out incremental improvements in the way we handle and render large MR diffs.

Recently we made the loading happen in batches which resulted in a significant improvement in how fast we start showing the diffs. https://gitlab.com/groups/gitlab-org/-/epics/1816

We plan to continue improving the performance of the MR page, lowering memory footprint this Q2 as well as providing other ways to enhance the experience.

Some examples:

* Commit navigation, to help commit by commit review: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28596

* File by file mode of reviewing MRs, including the concept of "unread diffs": https://gitlab.com/groups/gitlab-org/-/epics/516

We're constantly looking to improve and learn more about the struggles of our users. If you have further thoughts, please drop a comment in one of the issues and epics above and tag me (@andr3).

If some of your problems are not covered by these, feel free to open an issue (https://gitlab.com/gitlab-org/gitlab) and tag me as well.

Again, thanks for the feedback.


Hi! I'm an Engineering Manager at GitLab in Source Code team.

This is on our horizon. We have an Epic to track an overall revamp and adding an option to review file-by-file which will include developing something like what you just described.

Epic: https://gitlab.com/groups/gitlab-org/-/epics/516

Issue for the checking of file seen (including the concept of "unread diffs"): https://gitlab.com/gitlab-org/gitlab/-/issues/24629

If you could please add your thoughts in that issue, we could use that feedback while we develop the feature.

Thanks a bunch for caring and voicing your concerns. It helps us get better.


Thanks a lot for taking the time to answer! I hope my criticism didn't come off too strong, overall I love working with GitLab, the parent just struck a chord with one of my pain points. Glad to see you're working on it!


I agree it's very light, but isn't this what you mean? https://conventionalcomments.org/#format


I think "passive-aggressive" is a bit too strong. If you leave a comment, even if not blocking, resolving that discussing can simply be a signal that the author of the MR has acknowledged it.

The proposal of conventional comments simply clarifies intention and disambiguates it. It's like that salt and pepper question. How do you know which is in which? Regardless of your knowledge and assumptions, it depends on whoever filled them in. This clarifies that.


That's a great point, @tikkabhuna!

It seems like fixing that problem you describe could be a good way to improve the discoverability of the mentioned Badges area.

I've taken the liberty of documenting this in an issue [1] and putting forward a proposal. Would something like what I wrote there solve this problem for you?

If you have any more feedback to add, feel free to drop a comment.

I work as a Frontend Engineering Manager at GitLab and will raise the visibility of this issue to the right folks to get this on their radar.

Thanks for mentioning this.

--

[1] https://gitlab.com/gitlab-org/gitlab/issues/35771


Thanks andr3. That captures my thoughts perfectly. :-)

Its great to see such an open company. My boss and I frequently check Gitlab issues and look forward to release on the 22nd. :thumbsup:


@clarry, thanks for your comment. It keeps us on our toes.

I'm a Frontend Engineering Manager at GitLab for specifically the area of Create, which is where your screenshot falls under.

We do value convenience and good experience. We're in the final steps of preparing a significant improvement to the Repository Browser [1]. We steered away from things like virtual scrolling exactly for the reason you mention: it breaks the browser's built-in search functionality.

We're currently putting the repository refactor up to feature completeness with a plan to make this as fast and useful of an experience as possible on a modern browser. Navigating between folders is much swifter than if we didn't use any JavaScript at all, just to name one of the benefits. Another benefit of using JS here is the fuzzy file finder (press t on any folder and it pops up). All in the sake of convenience. We hope you'll appreciate the efforts we've put into these details.

Feel free to drop your thoughts on the Epic or creating new issues. Tag @andr3 on them and I'll follow up. We're listening.

[1] https://gitlab.com/groups/gitlab-org/-/epics/159


https://twitter.com/gitlabstatus/status/1003439258814877697?...

This link is for the monitoring page. The imports are going through.


Only good can come out of this. Looking forward to seeing this in action.


Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: