Code Review 14.0 Planning
Release planning for the Code Review Group
Overview - 14.0 Release Board
Team Weights
- FE Weight: w24
- BE Weight: TODO
Checklist
- Refine the issues already assigned to the current milestone for correct label hygiene
- Check/update past milestones for wayward issues
- Add vision items
- Add security items
- Add bugs
- Carefully check planning priority items for customers are scheduled or well communicated
Designs
Relates to
- #1282213.12
+2 - #1286514.1 +2
Activity
changed milestone to %14.0
added devopscreate groupcode review sectiondev labels
marked this issue as related to #12822 (closed)
Kai's running list of items for 14.0 (it's like a notepad)
UX Improvements
- gitlab#18458 (closed) - Sort MRs by merged and closed dates
- gitlab#281947 (closed) - jump to next thread tracking position
Features
&5641 - Persist viewed status
@andr3 @mnohr @phikai as usual, here's the Mural for us to add the issues we want to see in 14.0. I scheduled our 50-minute session for May 13.
- Add your issues to the Mural before the call. Let's try to limit to 5 issues per person, so it's easier to vote on them and keep things focused. You can find instructions on how to add them in the “Outline” panel on the right side of the Mural UI.
- Try not add Security or Availability issues. This is also noted in the product processes page, as those issues have forced prioritization with SLAs/SLOs.
- If you can, mark issues that appeared in previous sessions by changing their sticky color to orange.
Thanks and see you soon
Edited by Pedro Moreira da Silva@andr3 I noticed that at the same time on May 10 there's am “Application performance session” in the Code Review group calendar, but it was not on your personal calendar. Should I reschedule the prioritization session?
@mnohr how was your first prioritization session? What did you think went well, not so well, and that we can improve?
@pedroms Thanks for asking. It was a good experience.
Going in I was a little unsure about what types of issues we wanted to talk about during the session. It seems like to focus of this session was to cover issues that may fall under the radar with the rest of our planning process. For example, we didn't talk much about the OKR FY22 Q2Large MR issues, because we know we are going to work on those. There are also issues from %13.12 that may slip, security issues, performance issues, etc. that will take priority over anything we discussed during this session.
What are the next steps for the 4 issues we identified as the "winners" of this process? We agreed they are the most important of the issues we discussed, but how do we now balance these 4 issues with all of the other work we want/need to do in %14.0. I think that is part of this process that I am still unclear on.
@mnohr apologies for the late reply. Thanks for your feedback!
- Scope: Yes, you're spot on. There is work that we will have to do regardless (the kinds of issues you mentioned), and that's why we didn't talk about it during this session.
-
Prioritization: That's a great question! We raised these issues and discussed them because we feel they matter. There's no guarantee that they will be scheduled. At the end of the day, how they are prioritized over other work is @phikai's call, so I'll let him talk about it
What are the next steps for the 4 issues we identified as the "winners" of this process? We agreed they are the most important of the issues we discussed, but how do we now balance these 4 issues with all of the other work we want/need to do in %14.0. I think that is part of this process that I am still unclear on.
@mnohr it varies and was really my big question when I joined the group and we started this. In many cases we try to fit the items in and include them as part of planning. In other cases there's simply no room based on all of the other commitments we have.
I wonder if we should add a label to those so we can try and make sure we're actually getting through them.
-
I am going to do a first pass at data gathering for %14.0 here. On the Kanban board there are a number of other issues in the "Open" column that I am not sure what to do with yet. They will need more investigation.
14.0 Backend Capacity
w34 (Includes 10% Time)
13.12 Issues at Risk
These are Deliverable issues that have not been started yet and are at risk of not being completed in %13.12 (w15):
- w3 gitlab#325691 (closed) workflowready for development
- w1 gitlab#327308 (closed) workflowready for development
- w2 gitlab#218570 (closed) workflowready for development backend waiting for frontend capacity to work on this
- w3 gitlab#218410 (closed) workflowin dev
- w3 gitlab#281162 (closed) workflowin dev
- w3 https://gitlab.com/gitlab-org/gitlab/-/issues/211976 workflowin dev - possibly workflowin review by 2021-05-14
This issue was split into multiple smaller iterations:
InfraDev Issues
Performance
These could potentially be related to the Large MR work
-
w3 gitlab#209784 (closed)Moved to %Backlog - w3 gitlab#301204
Application Limits
Security
-
w3 gitlab#295298 (closed)Moved to %14.1
Edited by Matt Nohr
VS Code Extension
Top Priority
- gitlab-vscode-extension#344 (closed) - This unblocks the ability to create new comments in VS Code and prevents a dependency chain on compatibility with GitLab needing a bug fix in GraphQL.
- gitlab-vscode-extension#342 (closed) - Following the above issue, this actually allows new comments to be created
Secondary Priorities
- gitlab-vscode-extension#168 (closed) - as we further invest in MR/Issue views duplicate tabs are a real problem here and we should fix this
Remaining Capacity
Edited by Kai Armstrong@viktomas Tagging you in on this. I haven't taken a look yet, but any thoughts you have on planning for 14.0?
I would like to start working on MR Review: create a new comment on the diff as soon as gitlab#325161 gets fixed.
(EDIT: it's going to take some thinking to find out how to limit the "new comments" feature only to GitLab 14.0 and higher (where the bug is fixed))
Before that, there are a few smaller tasks that would be useful to the users:
Edited by Tomas Vik-
/cc @phikai
@viktomas Is gitlab-vscode-extension#344 (closed) still an option if we don't have that fix? Would that also eliminate a hard requirement on a release with that being fixed?
-
@phikai Yes, if we implement the workaround gitlab-vscode-extension#344 (closed), we'll get unblocked and we won't require GitLab version
14.0or higher
@viktomas I think for the sake of keeping this moving forward we need to go that route. The added bonus is we don't need a version with that fix. Do you think we can do the workaround and commenting in the same milestone?
@viktomas @andr3 I've updated #12823 (comment 572822552) with the three issues we should put in. That takes us to a weight of 8, do we think that's achievable?
-
Do you think we can do the workaround and commenting in the same milestone?
Yes! I'll start with the workaround and then proceed with the unblocked "new comments". The only requirement is that one is implemented after the other. I'm so excited!
That takes us to a weight of 8, do we think that's achievable?
I think that the new comments are "as good as done™"
@mnohr @andr3 I'm a bit concerned that we don't have any issues, even investigations up on the board with our Large MR OKR Label.
Can we please try to get issues created - even if it's the things we're going to investigate so that we can appropriately plan capacity against the other asks of the group.
@phikai thanks! Added a couple as did Matt. Currently sitting at 10 issues besides the ones for 13.12.
-
Frontend Weights
- Total team capacity: w30
- Slippage: w6
- OKR/tech debt work: w0. (As our OKR is aligned with product + lots of slippage, we're keeping all capacity to be decided together)
- Capacity for new work: w24
Slippage breakdown
- w3 NOT filter operator does not work for several filters gitlab#218570 (closed)
- w1 Add a boolean column for tracking MR draft status gitlab#330274 (closed)
- w0 Further improve Total Blocking Time performance metric of Merge Request Changes page with large amount of changes gitlab#327635 (closed) (waiting for virtual scrolling to be enabled and measure the improvements)
- w1 Merge Request Diff Incorrect https://gitlab.com/gitlab-org/gitlab/-/issues/211976
- w1* Unable to create multiline comments gitlab#322188 (closed) — i'm still waiting for a full report of the status for this, but it seems like it might be backend only.
-
w3 NOT filter operator does not work for several filters gitlab#218570 (closed)
Moved to 14.1 so we have extra w3.
Capacity for new work: w27
mentioned in issue gitlab-com/Product#2521 (closed)
-
Status Update 2021-5-19
Issue Health Status Notes MR Review: Introduce global MR cache 
This has slipped from 13.12. 4 MRs are merged, I'll submit the last one for review today. MR Review: track changed lines on the diff 🟢 I'll start tomorrow. This issue will work around a GitLab API bug gitlab#325161 MR Review: create a new comment on the diff 🟢 Opening the same MR or Issue in webview multiple times causes duplicated tabs 🟢 Stretch Other things
- With gitlab-vscode-extension!253 (merged), we removed the
execamodule from our production code. We now fully rely on VS Code for running ourgitcommands and we shouldn't have any more security issues related to executing arbitrary code.
- With gitlab-vscode-extension!253 (merged), we removed the
Status Update 2021-05-25
Priority Issue Health Status Notes 1 (slip) Improve Error occurred while fetching"useless" error message🟢 Currently on the end of review cycle #3, I'll likely be sending it to begin cycle #4 soon. 2 (slip) :spy_tone1: Confidential issue re: diff display 🟢 Not linking the MR here for reasons, but it's just finished cycle #1, and I'll be sending it back for #2 hopefully today. 3 MR diff HEAD comparison conflict warning 🟢 4 Use boolean for MR Draft status 🟡 
*Note 1 5 Prevent author from approving own MRs 🟡 *Note 1 6 Brainstorm parts of the MR page that can be cached 🟢 
This is essentially brainstorming, so I'll be thinking about it the whole milestone
*Note 1: I've marked this issue's health as at risk because the frontend work is pending completion by the backend. The theory is there won't be front end work at all, but we might get into a situation where the back end completes late in the milestone, with very little time for the FE to ship if we do need to do something.
Status Update 2021-06-02
Priority Issue Health Status Notes - (slip) Improve Error occurred while fetching"useless" error message🟢 1 (slip) :spy_tone1: Confidential issue re: diff display 
A huge amount of tests are failing in this MR and on master, so I'm stumped as to how to get this to pass the pipelines and get through review.2 MR diff HEAD comparison conflict warning 🟡 - Use boolean for MR Draft status 🟢 Many thanks to @kerrizor for shipping the backend part of this that magically handles this implicitly without changing the frontend behavior! - Prevent author from approving own MRs 🟢 Many thanks to @kerrizor for shipping the backend part of this that magically fixed the frontend display! 3 Brainstorm parts of the MR page that can be cached 🟢 This is essentially brainstorming, so I'll be thinking about it the whole milestone Edited by Thomas RandolphStatus Update 2021-06-11
Priority Issue Health Status Notes - (slip) Improve Error occurred while fetching"useless" error message🟢 1 (slip) :spy_tone1: Confidential issue re: diff display This just finished the third review, and will be going to maintainer soon. 2 MR diff HEAD comparison conflict warning - Use boolean for MR Draft status 🟢 - Prevent author from approving own MRs 🟢 3 Brainstorm parts of the MR page that can be cached 🟢 Status Update 2021-06-15
Priority Issue Health Status Notes - (slip) Improve Error occurred while fetching"useless" error message🟢 - (slip) :spy_tone1: Confidential issue re: diff display 🟢 1 MR diff HEAD comparison conflict warning - Use boolean for MR Draft status 🟢 - Prevent author from approving own MRs 🟢 2 Brainstorm parts of the MR page that can be cached 🟡 Edited by Thomas Randolph
-
Status Update 2021-5-26
Issue Health Status Notes MR Review: Introduce global MR cache 🟢 Merged 
MR Review: track changed lines on the diff 🟢 Merged MR Review: create a new comment on the diff 🟢 
I started working on this yesterday. It's progressing well, maybe even better than expected. Opening the same MR or Issue in webview multiple times causes duplicated tabs 🟢 Stretch I think I'm going to have the time to finish this. Other things
- You can learn a few tips and tricks about GitLab + VS Code in the new blog post Visual Studio code editor: Eight tips for using GitLab VS Code
-
Status Update 2021-06-02
Issue Health Status Notes MR Review: Introduce global MR cache 🟢 Merged MR Review: track changed lines on the diff 🟢 Merged MR Review: create a new comment on the diff 🟢 The main feature is merged in mainOpening the same MR or Issue in webview multiple times causes duplicated tabs 🟢 Stretch I think I'm going to have the time to finish this. Other things
- Our frontend team is at capacity, I feel bad adding work to people's plate with MR reviews (thanks so much for your time @jboyson and @thomasrandolph
- I fixed a small snippet bug gitlab-vscode-extension#389 (closed) and found a bigger one related to renaming all branches from
mastertomain: Inserting snippets is broken for newly created snippets - I had an idea for a feature that would allow you to
- Our frontend team is at capacity, I feel bad adding work to people's plate with MR reviews (thanks so much for your time @jboyson and @thomasrandolph
cloned to #12865 (closed)
marked this issue as related to #12865 (closed)
Holy smokes, totally forgot to keep this updated.. mae culpa!
Status Update 2021-6-04
Issue Health Status Notes Remove support for /wipquick action🟢 Ensure /draftquick action and "the button" will flip draft status🟢 Add drafts scope to MergeRequest 🟢 "Prevent MR approvals by the author" is ignored unless checked, saved, and unchecked 🟢 Add data migration to flip draft status column false->true for all open wip/draft MRs 🟢 In final review Rearchitect / Refactor MR Widget [Discovery] 🟢 
Making slow progress on defining next steps Add index on merge_requests for 'draft' 🟡 This may not be worth enough to add at this time - in discussion with DB reviewers Update source of draftto MR Grape entity
Moving to %14.1 since it relies on a post-migrate backfill Remove WIP/Draft regex title parsing Moving to %14.1 since it relies on a post-migrate backfill -
Only been a few days since updating, but I want to get back on "schedule" here with early-in-the-week updates, as that fits my work mode better.
Status Update 2021-6-08
-
Ok, we're almost at the end here.. another release in the books!
Status Update 2021-6-14
Issue Health Status Notes Remove support for /wipquick action🟢 Ensure /draftquick action and "the button" will flip draft status🟢 Add drafts scope to MergeRequest 🟢 "Prevent MR approvals by the author" is ignored unless checked, saved, and unchecked 🟢 Add data migration to flip draft status column false->true for all open wip/draft MRs 
UPDATED - the sheer size of the merge requests table causes timeouts on staging, so back to the drawing board, and... 3 different DB engineers and no one really knows the path forward. This is slipping into %14.1 now, which pushes a number of other WIP -> Draftcleanup steps into %14.2.. I don't like it either.Rearchitect / Refactor MR Widget [Discovery] 🟢 Closed and moving on to planning individual steps diffs_metatdata Returning 500 Error - nil can't be coerced into Integer 🟢 MR merged in the last 12 hours. There's still an underlying issue.. but at least we're not erroring! Replace work_in_progresswithdraftin API🟢 Merged Unable to create multiline comments Investigating 100% slipping - I've been fighting DB migration headaches, haven't had brainspace to start this Add index on merge_requests for 'draft' 
Closed This turned out to be not worth enough to add at this time Update source of draftto MR Grape entityMoving to %14.1 since it relies on a post-migrate backfill Remove WIP/Draft regex title parsing Moving to %14.1 since it relies on a post-migrate backfill Edited by Kerri Miller
Status Update 2021-06-14
Issue Health Status Notes Warn users that they should use supported GitLab version 🟢 
Remove WIP/Draft regex title parsing 
Blocked. Waiting for BE. Sync show whitespace changes in diffsuser preference🟢 Unable to edit squash commit message when squashing is required in MRs 🟢 Display change that made thread outdated in-line Blocked. Unable to create multiline comments Blocked. Edited by Justin Boyson
Status Update 2021-06-08
Issue Health Status Notes Merge Request Diff Incorrect 🟢 Ready to be merged It missed deadline for 13.12 security release so it will be merged in 14.0 security release. Load merge request diffs incrementally from Gitaly 🟢 I believe the backend change is mostly ready to go, but we have some edge cases where we make an additional full diffs request which I believe needs some frontend involvement. It's only an issue when we have files that have whitespace changes only, but I think we need to address that issue. Improve performance of diffs_batch action for Projects::MergeRequestsController under load to target Not much has been done other than initial investigation. Status Update 2021-06-11
Issue Health Status Notes Merge Request Diff Incorrect 🟢 Ready to be merged Load merge request diffs incrementally from Gitaly 🟡 I have a work around in place for the issue mentioned above so hopefully we can ship it soon after a final round of review. Improve performance of diffs_batch action for Projects::MergeRequestsController under load to target Other things:
-
[Feature flag] Rollout of
notification_setting_recipient_refactor. It's a simple FF removal and it is being reviewed currently. - Draft: Remove diffs gradual load feature flag. It's a FF removal which requires more attention due to many failing specs. I just picked it up as I noticed that it was default enabled several months ago. I took over an MR @iamphill started earlier and fixing failing specs in it. It was making diffs_batch code more complicated so I'm getting that FF removed while I'm at it.
-
[Feature flag] Rollout of
-
Status Update 2021-06-09
Issue Health Status Notes MR Review: Introduce global MR cache 🟢 MR Review: track changed lines on the diff 🟢 This turned out to be the hardest issue, I had to go back and fix a few edge cases MR Review: create a new comment on the diff 🟢 Opening the same MR or Issue in webview multiple times causes duplicated tabs 🟢 1 MR in review, 1 more to go Other things
- Tanks @jboyson and @iamphill for the quick reviews
- I fixed a small regression fix: validate CI command didn't show validation result
- I fell in love with TS again when introducing
no-floating-promiseseslint rule to the codebase. Requiring each promise to be handled is better than sliced bread. - GitLab Discussion API has sometimes troubles submitting slash commands in new comments (gitlab-vscode-extension#357 (closed), gitlab#326443 (closed)) I'll keep an eye on production logs to see if there's an influx of errors when creating comments. But so far it looks like roughly 1/day.
Edited by Tomas Vik - Tanks @jboyson and @iamphill for the quick reviews
marked the checklist item Carefully check planning priority items for customers are scheduled or well communicated as completed
marked the checklist item Carefully check planning priority items for customers are scheduled or well communicated as incomplete
added Planning Issue label
-
Status Update 2021-06-16
Issue Health Status Notes MR Review: Introduce global MR cache 🟢 MR Review: track changed lines on the diff 🟢 MR Review: create a new comment on the diff 🟢 Opening the same MR or Issue in webview multiple times causes duplicated tabs 🟢 Other things
- I added maintainer documentation for the extension(docs: maintainer documentation)
- I found a small bug in MR Reviews (Context menu shows when trying to add an MR comment + duplicated comments) I submitted a fix for review today.
