This issue is for feedback on the new merge request homepage. Details about this new feature can be found in the documentation.
How to give feedback
Check existing feedback & known issues: Before submitting, check to see if your feedback is already captured in the known issues section or reported by someone else in this issue. If so, comment on the existing thread or leave an emoji reaction to show support.
Start a new thread: If your feedback is not listed, start a new thread with a descriptive title. Include relevant details, screenshots, and steps to reproduce the issue in expandable sections.
Be Specific: Provide as much detail as possible, including device/browser information, steps to reproduce, and expected vs. actual outcomes.
What you can expect from us
We will read all of your feedback.
We may not respond to all feedback directly.
We will create issues for repeatable bugs and assign a priority based on severity.
Known issues, feature gaps and changes in behavior
As we work towards maturing Duo Code Review for a wider release we'll track gaps and changes based on the feedback we receive as part of this internal test.
@spencer.landis Can you share more on why you think it's a downgrade? What value were you getting from knowing all of the items you were involved in, even if you didn't need to explicitly work on those?
Just a quick reference of how much open work there is. Even if one I'm assigned hasn't received feedback yet, it is helpful to know I need to bump a message about it to the rest of the team.
@phikai Yes. Not just "my own work", but in general I would not assume that the person reviewing has the same responsibility as the person integrating. Reviewing involves code/logic of an implementation/change (i.e.: developer). Integrating typically involves CI/CD pipeline executions and sometimes that may require further consideration to decide how or when to merge it exactly, or in which order if there are multiple approved MRs pending integration (i.e.: product owner).
@phikai Many times they are the same person, but not always. And MRs can have their assignee changed. The assignee is the one responsible for handling the MR and should be considered active for them until it is either merged or closed. A MR can be considered inactive for everyone if there is no assignee (i.e.: a MR, independently of whether it is ready to merge and/or approved, could have their assignee unassigned if for now it is not considered a priority anymore).
A better solution will be to let the user select what to add to the counter.
I also miss seeing at a glance how many open MR's I have and how many I need to review.
I am used to starting the morning by clicking on the counter and seeing if the number has changed. This way, I know I have a new review waiting for me.
This new layout is terrible. I just want to see all the review requests that I am the assignee on and now it is split up!
Also, the section Waiting for assignee literally isn't that. It is merge request that I have commented on that have not had a response to every comment.
Also, if you collapse sections and then refresh it doesn't save the state of what was collapsed. So you still have to open/close sections that may/may not be relevant to you.
And "Assigned to you" are requests that don't have reviewers for some reason. Sometimes I have review request with no reviewers because I want to kick off CI, they aren't requests that I want front and center, certainly not before requests that I have assigned reviewers that don't fall under the active count.
The one thing I did realize is that you effectively can get the old view back by using Search and searching for MR's that you are either the reviewer or assignee on. There just isn't a button for exactly that anymore.
Personally I really like this =)
There are work/improvements still to be done, but this is a very good start I think =)
It is also nice that only have to click once to see all MRs in one view instead of twice to either get to those Im assigned to myself, or the ones Im reviewing, that was very annoying
I agree with Matt! I don't understand the splits and this makes it harder for me to find the PR:s that I am looking for. I don't need all the extra information attached and don't understand what half of it means.
I think the main issue for me is that I want to be able to find the PR:s by name. And the name is a lot less prominent in the new design. The most prominent now is the "PR status" which is pretty useless for finding a specific PR. Id much rather have a basic list like it used to be and then have the status listed right next to it. The changes make no sense to me.
I don't need all the extra information attached and don't understand what half of it means.
@simon147 Did you look at the documentation for the new homepage? Did that help you understand more about the information on the page? Are there aspects that still aren't clear?
I think the main issue for me is that I want to be able to find the PR:s by name. And the name is a lot less prominent in the new design. The most prominent now is the "PR status" which is pretty useless for finding a specific PR.
What are you looking for when looking for a merge request by name? Is there something about the name that would tell you to take an action or are you just looking for specific ones because you know some other way that you need to take an action?
Rather than selecting to view where I am assignee, I now have to scroll down quite a ways to find my MRs past a bunch of MRs that I am marked as a reviewer or collapse the section, though this collapse doesn't persist adding extra steps every time. The ability to search or filter results is also gone with the removal of the search bar. This change offers no benefits and several obstacles to my workflow.
The ability to search or filter results is also gone with the removal of the search bar.
@AndrewBranoff The previous implementation with a fully featured filtered search is still available on the Search tab. Also, if you follow an old bookmark, there are redirects in place to take you there. Thanks for the feedback.
The usual goal of software improvements is to lower the number of clicks and steps needed to make use of it, adding an extra step, clicking to the search tab, makes it more difficult to use.
@AndrewBranoff Can you say more about why you're looking for MRs where you're the Assignee if they're not surfaced in the top sections where you'd need to take an action? Is there some other signal you're getting that tells you to take an action on those?
@phikai Much like Miguel in Spencer's comment thread, in my organization (or at least in my team at my organization, others may operate differently), the assignee and author of an MR are usually the same person, and that assignee is responsible for the MR through the rest of the process, including the rebase and final merge. This new format pushes down MRs that are ready for that next step in the process of merging in, and does so in an odd way, since I see MRs listed in the section titled "Waiting for approvals" that have all required approvals for merge hidden in with those that still need approval. (Practice here is that everyone on the team will be assigned as reviewers, but only two are needed to approve before a merge, meaning most MRs have 3 reviewers assigned that never see the MR above the 2 that do on the 6 dev team)
I don't like the new layout at all. I like the old split between Assigned and Review Requested very useful and better than the new one to organise my work.
Where are the filters for quick search of past merge requests ?
Was super useful to have it at the top of the page, instead of an other tab.
Also, I'm an assignee for one merge request I created (yes this is how we use it here) and it doesn't show up in the "Assigned to you" section. Why ?
I'm an assignee for one merge request I created (yes this is how we use it here) and it doesn't show up in the "Assigned to you" section. Why ?
@bthery_altair Does this merge request have any reviewers assigned to it? If you are an assignee and it has no reviewers it will show up in the Assigned to you, if it has a reviewer assigned then it'll be in another list depending on the review state.
"Assigned to you" was missing 2 MRs of mine because they were in the section below the fold called "Waiting for approvals", which was a bit unexpected.
Really, "Assigned to you" means "Assigned to you and the next step is for you," I think @bthery_altair .
The old pages showed labels put onto MRs, which was really helpful to know which MRs are high priority potentially, or anything else. They were a good way to get additional information beyond the approved/not-approved detail at a glance.
Could labels be displayed on the new page with each MR entry?
@Matthew.Rupchan In some of our design iterations and tests we did have toggles that allowed label configuration. We ultimately went away from that, but I've opened #517583 (closed) so we can continue to explore that.
Why would you do that? You claim in the new merge request popup "Know at a glance what merge requests need your attention first", yet you provide less information?, now i have to go to the search tab to actually see the useful information. Who thought that would be a good idea? Why break something that worked ok?
@phikai I cant answer for Michelle, but we use it to label different mr for different groups, and also prioritization and similar info. It helps a lot making quick decisions on which MR to look at now, and which are ok to wait a bit with. For example
Another thing I noticed now is that we have automatic systems (dependabot) that creates lots of MRs with newer versions that we need to start to use. But sometimes we cannot do it directly and add "on_hold" labels, or other "grouping labels". But when dependabot creates more MRs and it is 20+ MRs in the list, we really want to see which we have marked before so we do not have to go into each MR all the time.
At least you've done your research before making such changes to understand user's values and you know that simple > bloated. Oh wait.
Looks like another change in a series of changes completely separated from reality (to be honest, there were no usefull changes in Gitlab's UI for quite some time already).
If I'm assigned to a merge request and it's not either merged or closed, I want that MR to count as active, as it usually means that it still requires my attention in some way and I need to keep tabs on it.
On the other hand, I like that merge requests that I am reviewer in and have already approved are now no longer included in the active count. (I assume that if new changes are pushed and my approval is reset, that merge request will again revert to counting as active?)
as it usually means that it still requires my attention in some way and I need to keep tabs on it.
@pekka.korhonen Can you elaborate on what kinds of things would require your attention on those MRs? If they're awaiting a review, for example, when the review is finished it will return to you and then be part of your active count since it's ready for you to take action.
@phikai Reviewers might leave comments, questions and/or suggestions before adding their approval. Also, even when there has been no action at all from the reviewers, I will sometimes need to remind them to take a moment for code review - they might simply be busy with other tasks and forget otherwise.
Until now it has been convenient to go through the list of MRs assigned to me every once in a while to address any discussions and code changes, but also if there has been no action from the reviewers to remind them that I want to move this feature along and they need to take a look at it before I am able to merge it.
Can you elaborate on what kinds of things would require your attention on those MRs?
An example is "go chase the reviewers to actually review it". I'm not sure it would be an acceptable answer when asked why my feature still isn't merged a month later to be "well, gitlab didn't say I had any actions to make on it so I kind of forgot about it"