HN user

andr3

86 karma
Posts7
Comments14
View on HN

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.

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 Gitlab 7 years ago

@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

Having been bit by this warning message, all this sounds a bit too familiar.

At the time it was being caused by McAfee (yes, dust-off the anti-virus conspiracy theories) had flagged our domain as untrusted because our main virtual host (www.) was returning an HTTP 200 on a 404 Not found page. Yes, that's the "security risk" they found. sigh

I find myself treating influads ads as content in the websites I find them... and that's gotta mean something.

In the end, and for future projects, I'd definitely be interested in trying out these alternatives... Google is spitting on our peripheral markets. It's a known fact that the most willing to pay for ads aren't the most valuable, we know that from the amount of ringtone-selling-companies to fart-machine-apps stuff we see being delivered by google's network.

I say it's time for companies to help each other by joining a marketplace where their ads are treated as valuable inventory. They know their ads won't share space with fart-machine-apps and alike.

I like this tiny change, both as a developer and a user.