GitLab Community Office hours calls, 7 Jun 2022

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: How GitLab is Triaging and reviewing Merge Requests - Community Office hours Call June 7th 2022

Description

June 7th, 2022
During this Community Office hours call, Brian Williams is walking us through the process of training and reviewing community merge requests.

Meeting's Agenda: https://bit.ly/3y8fqZc
Contribute to GitLab: https://about.gitlab.com/community/contribute/development/
Join the next office hour call: https://www.meetup.com/gitlab-virtual-meetups/

A

Hello um welcome hello. Everyone uh wishes for a great gitlab day, wherever you are I'm christos and along with all these wonderful people here today, uh we're here to welcome you in the another office article I will um with some new faces. So why don't I just like open the floor for some really brief introductions, and then we can dive more into the freestyle agenda that we have for this office article.

A

So I'm christos, I'm the contributor program manager and my role is to help you all contribute have a rewarding experience um and yeah make them contribute to the open source ecosystem and, more specifically, uh to get love open towards my passion. So yeah, I'm really happy that everyone is here for for the same reasons.

A

So next to my list here, I'm going through I'm gonna, I'm gonna call out people if you cannot um unmute um feel free to type into the chat uh since marco is here and on face muted, and I will invite you, marco, to introduce yourself.

B

Yeah I'm well, I'm marco and I start contributing to github couple of months ago, and I basically done various things from backhand to front-end and yeah. I really enjoy contributing, so I keep on doing it and that basically it.

A

Wanna call someone.

B

Else, I don't really know.

A

Angel we're gonna go next.

C

Okay, uh in show text in the chat. I guess they can't speak but uh and schulz says oh currently in a meeting- a full stack, developer and love contributing to.

B

Oh yeah, by the way I I am no way a web developer. I almost work basically everything in 3d environment and 3d engines. So web development is not really my thing, but everyone can contribute right.

C

Yeah gitlab has gillam has a lot of things that aren't web. um Like you know the runner and there's a kubernetes agent and there's a few different projects like that that aren't really web but still fit into the ecosystem.

B

Also, if there's something on gitlab that you might find useful and that's not really working how how you would like it to or maybe if it's missing some features, you can that's why I started contributing. So that's a good reason to start.

B

C

Think yeah so I'll go ahead and introduce myself since I haven't gone yet uh I'm brian and I'm a back-end engineer on the container security team. uh I work on container scanning mostly, but I do kind of float around in the secure area.

C

I'm also a merger quest coach. So uh I will I help out contributors with uh their contributions. So if you ever contribute to getlab, uh I might get assigned to help you out with around your quest.

C

Fatima, do you want to go.

D

Yeah, I'm fatimah. I am a developer evangelist, I'm with christos on the community relations team. I work on managing our community forum and some of our third party tools and just kind of engaging with the community and figuring out what they're talking about and what they'd like to hear about. um I think this is the first time I made office hours because I'm on the east coast for two weeks, and I can finally make the time so excited to be here and learn from both of you.

A

Thank you, mahul. Do you wanna introduce yourself.

E

A

Oh yeah hi am I audible.

E

Yes, okay, so yeah pretty much a similar story. As to marco, I started contributing maybe a year back to gitlab. You know having fun and stuff like that, like I have some mrs left over, so just need to complete them. I guess so yeah just hopped on in this call and pretty excited.

A

Perfect, thank you um before um I have just a two really small topics I would like to cover, and then we can um ask, maybe brian, since uh they are here um a few things if they want to or show us like last time how to contribute to gitlab. It was really nice. uh But what I know why don't I share my screen really quickly. um Where is it?

A

Okay, first and most important um contribution themed months, so we started introducing the contribution themed months a few months ago. It's like um six weeks now um and it used to be a one month, but now we extended it to six weeks because we understood that setting up your developer environment, it's take some time and figuring things out, so we are organizing, like we have what six weeks focused on one contribution area, pretty straightforward.

A

This time is backend, so the idea is like we're trying to have these office articles on a weekly basis, trying to invite gitlab engineers, back-end engineers, specifically maintainers, to help us. You know to walk us through some of their thinking on how to approach and solving specific issues or just like giving us their insights on how to understand the code, for example, or approach approach, some bugs. In some cases we had a really nice call the other time with brian, and they are joining us again today, which is really nice.

A

Thank you and then we're working uh with other people to um we're gonna have more um maintainers joining us now in terms of like um how to find issues to contribute to on the contributing guide. There's this nice table uh some of our product groups. They have hand-picked issues uh for everyone um we have on the on the main.

A

On the second column, we have like the contaminated gitlab guide. We have the architecture and everything on the right side. We have some issues um and for from different product groups, you can explore them and advise them to work on, and if you need help the help section here, which is either by pinking um one of our mr coaches or joining our contributors, gitlab channel or just emailing.

A

But most of the time the most effective one is like uh doing a gitlab bot help and it's gonna notice and notify everyone and we're gonna have to help.

A

That's the contribution thin months, uh we're also gonna have the hackathon in the middle of august, but still we're wrapping up the previous hackathon still reviewing all the mrs now another thing that it's came into our attention about the cicd minutes usage. um So if you're contributing to gitlab- and you started running out of cicd minutes, we are investigating the problem there. But if you run out of ci cd minutes, please email contributors at gitlab.com or reach out to me directly on either.

A

Git lab or on guitar or on anywhere that uh we might all be together, we're still investigating, hopefully we're gonna fix it soon by the time you're watching this video. This recording the problem won't exist. Sorry for this issue is currently private. I'm gonna make it public. It includes some screenshots, so I want to make sure I want to take everyone's.

A

Approval before making this um this issue public, so that's it for my site, some house be keeping stuff um so yeah. um If anyone has any topics would like to discuss or any questions potentially for um for brian, uh it would be. You know. Now is your time um they are here with us, so yeah. Otherwise we can do like another round of picking issues or you know something else, brian like how you're approaching or how reviewing.

E

A

Nmr, I don't know that might be interesting.

B

I might have a small question that maybe he would be, he knows an answer, um and today why we were using doing our parenting session with lee. We were trying to find a way on how to debug and set breakpoints on sidekick jobs. Do you know how way to do that.

C

No, I've never tried this again. I've never tried deep background inside kick job before so uh that that does sound difficult. um I think what I would try doing is it should have.

C

It should have tests that are individual.

C

So what I would try doing is setting a binding deprive fit point and then try running the tests, and that should let you hit the breakpoint hopefully but um like, but like actually trying to debug it when it's running, um I don't think there's a practical way to do that, since it's asynchronous.

B

Yeah yeah, that's actually the problem, the that we were having because they, the tests that we were trying to run, didn't really cover the theme. The thing that we were trying to test so we tried to add binding, don't pry but and testing on the live gdk environment, so not from the tests, but it didn't really hit those those break points, so that was uh that wasn't successful. Unfortunately,.

C

Yeah we can make note of that, and I can ask uh and our beckon channel and maybe there's somebody who's more familiar with sidekick. Who knows a good way to do that? I'm not aware of any way to do that.

B

Yeah, I I don't know if maybe leah has already asked that question on your slack channel. So maybe he's already done that.

A

I'm taking the chat if anyone has any questions there or like to ask something.

A

A

A

Brian, um would you be interested, I don't know what uh maybe helping us walking us through a review process.

B

A

How to review an mr, I can get some if you want or want to pick another issue um to approach. I think we've covered that pretty pretty extensively last time and a really nice video as well, uh that we created based on your recording so um yeah, I'm just trying to think of ways or marco or ansvul or mehul what you'd be interested in. You know.

C

Yeah we can do a review. That sounds like a good topic to cover today: yep yep, yep.

F

C

So is this readable for everybody? Is the text yeah I'll zoom in a little bit just two, oh yeah, so.

C

There's some uncharged merchandise that we can look out.

C

Yeah so so, every day uh the coaches get one of these charge issues and all of the merch requests that aren't trying to get uh assigned to coaches so that they can apply labels and uh look for a reviewer. And I got to sign this one from bitesto.

C

They are a very prolific contributor.

C

One second, my captions, my captions, are on their screen, so I'm like.

C

C

First thing: I'm trying to do is understand what the change is.

C

There's not much in the description, but it looks like they're trying to get rid of jquery.

C

uh This is front end, so I'm not too familiar with this one, but I can still find somebody to review it.

C

So this so first thing is I'm looking at the location, and this is authentication code, so you've got u2f, which is like a ub key and web authentication, which is also um like a more generic api for two-factor authentication.

C

So this would belong to the authentication and operation authorization group. That's already labeled.

C

Yeah, it looks like it looks like somebody already looked at this.

C

And they already assigned a reviewer to.

C

Yeah, but there are, if you set up your pipeline, then you should get a comment from danger.

C

It looks like if I still hasn't set it up, but we have documentation on how to do that somewhere, uh dangerous reports. I think.

C

But this is a code review tool here it is and if you set this up, it'll suggest reviewers for you.

C

And so usually what most people do? Is they look at the reviewer that was suggested by gingerbrot and they use one of those? um So the bot tells you how to set that up.

C

But if you do not have it, then the way I would find a reviewer is to go to roulette.

C

So we have a thing called reviewer roulette uh like a rule roulette wheel, and you use this to randomly select someone to review your merger quiz.

C

And this is the tool that I use the most for selecting a reviewer.

C

Now you don't have to use this, you can, if you want to, you, can just do.

C

You can just ping the bot and you get airbot request, request review and the bot will peen a coach and the coach will find a reviewer for you, but um you're totally welcome to select a reviewer yourself too, um and if you know how to do that, it's it's uh probably it's more efficient than uh making a post finder, making a coach find a reviewer for you.

C

So if you use roulette, um you want to select the project up here and we're on gitlab, but all of the pretty much all together projects should be in here or most of them. So you should be.

E

C

One, whatever project you're working on here, like if you're working on runner or.

C

Get that agents in there a lot of the projects on that. So we have a right there's.

F

C

Bunch of different reviewer categories, depending on what you change and if you set up danger, it's able to automatically detect uh who needs to review the merger press based on what files you change, and so it will show all the different roles uh who need to review the margin quest.

C

We don't have danger here, but I know that since it's touching front-end files, then we need to find a front-end, reviewer and it'll show all the different engineers who are able to review front end here and you can, if you want to you, can click spin the wheel and pick somebody randomly, but what I usually like to do is I find someone who doesn't look like they're busy like who doesn't have a lot of mud requests assigned and I'll pick them so like daniel tian is out on parental leave. So don't want to pick him.

C

Yeah, a lot of.

C

A lot of the engineers on here don't have very many reviews um like sam beckham, looks like he has the most murderess right now. It looks like maybe he was uh out of the office or something he got a few accumulated. But you can look at. uh You can look at like the there's statistics, for how many merchants people get over time so, like doug stool, gets like two a day.

C

um So what I usually like to do is I like to pick someone who doesn't get a lot of murder quests, like jeremy jackson here and I like to request a view from them.

C

Yeah so then, so when you find someone you can just comment like yeah, his status is busy. So if, if their status is busy, you usually don't want to sign them, um it's not supposed to.

C

It's unavailable folks paid.

A

Can I ask something really quickly yeah sure? uh Is it this available for community as well.

B

Yeah, I can use it. Yes,.

C

B

C

It's a public website. I added a link in the agenda, so you can click on that and find it yeah. I have a bookmarked right over here, so I just go back to the bookmark. um Did danger also linked it.

B

Yeah a danger also, if you set it up on the comments that it will add to your request, it will add a link to to this page.

C

A

I was, I was confusing it with um the new um review roulette, which is like automatically trying to find um maintainers and assign them uh reviewers, which currently is not open for community marks. They don't they're not doing it for community wars. Thanks, sorry for interrupting yeah.

C

Yeah this page is this page is pretty much the viewer relay uh when dangerbot does relate. It actually reads data from from this website and it uses it to find our viewers. So there's a there's roulette.json- and this has all the data that's being used to populate this page and danger actually reads this, and then it randomly picks. Someone from this big json list.

C

Right so so, if you find once you find a reviewer, um you can just copy their username. I actually added this button. A few months ago,.

C

um And you can go over to your merger quest and pin them and just ask like hey uh semester. Can you please review this merchandise and then you want to assign them, I'm not totally sure. No, we can't.

B

C

If yeah contributors can't assign people, um I think.

C

It would be, it would be nice if you could use the box to assign somebody as a reviewer. uh I wonder.

A

I think we can um no, you can apply labels.

C

Yeah, I need to look into that, but it would be.

C

But it would be cool if you could do like get that uh and reviewer.

C

If we don't already have that, then I should. I should be about adding that. But for now you can do get about request, review.

C

And and then that'll that'll get you a review, so you know, since you cannot sign people do that for now, um but I think it would be really great if we could.

C

Make it easier for contributors to do this themselves.

B

Also, the roulette is also useful when danger bot tries to do his best to pick the best reviewer, but if you, for example, in a hurry and you need a review as soon as possible, you can use the roulette to find someone. That's I don't know if he's just starting in the morning, maybe he's a has more free time and you can ping him.

C

C

Okay, uh do y'all want to look at anything else, or do you think that's good like? Was that useful.

D

I thought that was great.

C

D

I didn't know about roulette, so that was really cool. um I don't know a lot about danger about, so I appreciate the life of you thanks.

C

Yeah, I I do want to show you another merger quest real quickly. uh One second give me give me a minute to find it.

A

A

A

I'm really curious from where you get the status for them on the gitlab roulette on the dashboard their status is, I think it's just like the status that you put on your github profile, yeah correct. So if someone is available or not available, you know it.

A

That's that's something that we always also like struggling, sometimes we're trying to assign people to intuitive years um and mrs and we don't know their availability and some people yeah. So it's always like a struggle, and it's also it's different from community on mars because all of these people review community mars but yeah, that's gold. Thank you. Brian, for sharing.

C

Yeah, so here's another merch request that I just want to show you real quickly. This is a real big one that was submitted by jonas over at simon and.

C

This was part he did some work to uh make topics you know on on a project. You can have uh topic labels that kind of integrate, indicate, search terms that you can use for that project that doesn't have any, which is interesting.

C

I was sure we had some topics on those, but anyway.

C

These are pretty much keywords that you can add on to your projects that you can search by, uh like you might put your language like ruby or go or whatever, and then it should it'll. Let it show up on the explore page so like here I can do getapp.com explore, explore project topic go, and this will show me some projects that have the code topic.

C

This used to be case sensitive. So if, if I did this with capital g, it wouldn't show me the same thing, but he changed it so that it's case and sensitive now, and that's what this this much request was for um did. This was one of several magic words that was used to change this and.

C

So I ran a pipeline and this is what the danger output looks like, so it shows all the different reviews. So this merger press touches on several different areas. You've got back-end database and front-end, and so you need to have reviewers for each of those and the way the view process works.

C

Is you get a review from a viewer first and that person will give you suggestions etc and once the reviewer approves, then they ask a maintainer to review it, and so you need a reapproval from each reviewer and you need to put an approval from each maintainer and once you have all the maintainer approvals, then the last maintainer to approve the merge request will merge it. uh So this one had three different roles that needed to review it and it also updated documentation.

C

So we had to get a technical writer to review it too. um But but technical writing reviews are non-blocking, so we can merge it without technical writer approval because they can go update the documentation later um but yeah. If you look at some of the files that were changed, real quick, uh we have some back-end here.

C

So, like the topics controller, uh some of the models so controllers, um if you're not familiar with rails, these are common rails concepts, there's controllers, which uh handle uh like the parameters and stuff from an http request and then there's models which interact with the database.

C

So each model is uh like an object, so topic uh project. These are, you know, object, oriented things and they each represent a table in the database. Pretty much then views are used to uh yeah. Typically, a controller calls a view in order to render html.

C

Then we have database migrations and these are used to make changes to the database schema. You can see, he's adding a column and then you add a text limit to that column.

C

Yes, so so, if you're, if your model depends on the database change, um you probably have to have a migration of two and that needs database review and then you've got the documentation, change the documentation that shows up on uh docslab.com most of these, not all of them, but most of them are located in the gitlab repository.

C

So you know, if you want to change this documentation page, you can do that through a markdown file and get them in the github project and if you scroll all the way down the bottom, there's a link that says edit this page and you can see it goes to get that document. Ldap index.

C

Am I going too fast.

C

Yeah and we've got an api entity these. This is pretty much an api response, so we're adding a new field to that api response.

C

And that's for.

C

You know similar thing to the api parameters. These are the primary issues in the requests.

C

uh Oh gloria background migration: um I'm not gonna talk too much about this, but this is uh this is the database migration that happens in the background? um So sometimes sometimes these can take like days to finish. uh Then the ko get there.

C

This is a big huge file that contains every single translatable string in gitlab, so every single one of these messages um it's possible to create translations for these. So pretty much every time you add a stream that the user will see.

C

um It's probably gonna end up in this file and we have a whole different program where people can contribute translations for these strings so like, if you know russian or german or like any other language, you can take these strings and add a translation in your language and it will start showing up in gitlab when you change your language settings the we we rely on contributions for that, for the most part.

C

Then we have the test.

C

And if there's actually not really any front end changes here, but pamela the views, the these are kind of um unique and that they're they are considered both running and back-end. So these two files are probably what is triggering the front-end review um and yeah. These are weird because they're they're a mix of both ruby and html and css.

C

uh So you know this is this: is a css class and then you're able to call ruby code from in here and yeah? I I don't understand it super well, but it's it's a hybrid thing, so that usually gets both back in and in front of the review yeah. So for this major quest, uh let me see yeah there's this picture. You said here where uh I started asking for views from everybody, um so yeah.

C

If your merch request is ready for review, it's usually a good idea to add a comment saying so or uh use get that bot request review because it's not always obvious if you're, if the magic person's ready or not- uh and you know I picked- I picked reviews for each of those and then the technical writer, a technical writer only needs one reviewer.

C

Most of the time we have two but for technical writing, there's one um yeah, and so so everyone kind of added their approvals in this thread, and you can see you can see when the reviewer approves they, they request a review from the maintainer, so they just kind of pass the baton.

C

And you know each of the and they're they're: either they'll either approve it or they will leave comments and ask you to look again and usually when they ask you to look again, uh you will have to request a review from the same person again after you make changes.

C

uh So if you, after you address the comments, you can just pin them and they will take another look at it. uh Some people like to use the ping pong pedal because you're going back and forth. So it's like you're, like you're playing ping pong.

C

Yes, so we got approvals from most of the maintainers pretty quickly uh front, end approved database approved, and then we were just waiting on back in uh the person who got picked for beckon review was out of the office, so we found somebody else and they went back and forth a couple times and then they merged it.

C

So yeah that that's what the review process looks like um most of the time. Your changes are to be small, so you might only have one or two reviewers, uh but this one was really big and you know. Sometimes, if you touch a bunch of different areas, uh you might have to have a bunch of people review it.

C

Does anybody have questions about that.

B

I might have a question about the having a single merge request, touching both backhand and front end, because lately I discovered that that is not a really good practice, because uh the deployment is not really synchronous, so it could cause some problems.

C

Yeah, it's sometimes you kind of have to do it.

C

Like if the funding is dependent on bathroom changes, but you're right that you're not really supposed to do that, because we have a thing called.

C

It's in the documentation somewhere, but there is.

C

I think what I know.

C

Configurations what it's called.

C

Zero downtime upgrades: this is what I'm looking for.

C

Yeah but you can.

C

This is like a special upgrade process where, if your gitlab instance is composed of multiple nodes, like you have multiple servers get you can update.

C

You can use this update process to upgrade those servers one at a time without taking hitlab down, and the thing about this is.

C

You might have servers with different versions running at the same time and if the front end expects so the front end and the back end might not be on the same version. When you do this, because the front end, you know, sends requests to the back end. It can hit any of these servers, and so you could be talking to two different versions of gitlab at the same time, on the front end, so we actually had a.

C

Backwards compatibility guidelines.

C

Yeah, this documentation uh tells you what you need to consider with that kind of stuff uh when modifying javascript.

C

Is it okay, when a browser has a new javascript code, but rails is running the previously the previous monthly release on the risk api, the fql api or internal apis in the controller? So this page gives you kind of questions that you can ask to determine if it's okay to do that.

C

So, if you're updating the front end and backing at the same time,.

C

C

If your front-end changes will still work without the back-end changes, then it's okay.

B

You can even set up the front end change to make sure that it still works with the old backhand to keep it. Keep it all. In one, much request.

C

Right yeah, but you could you could totally split those in separate merger. First too,.

B

Oh, I do that because I don't I don't want to make a mess request and then being asked to split it up. It is, can be a nightmare, so I prefer not to do that.

C

Right right, I know what you mean yeah it you know it's it's a judgment call so there's not really.

C

That you can't really always there's not really a guideline, then you can follow to always arrive at the same answer you have to you know, make a judgment call on it.

B

Yeah, the the sad big quotes there thing that I discovered when we talk about this is that a database change could take even three milestones to get deployed, because maybe you, if you have to change a column and or drop a column on a table, and you have to make sure that everything is updated and that could be take taken care of on three milestones. So three months.

C

Yeah, did you did you have a question for me? I kind of wasn't paying attention.

B

Oh no! No! No! No! So it was just a consideration.

C

A

Thanks um are there any questions for brian.

A

We're pretty much in time.

A

Soon, check here.

A

Let me check the chat if there is any questions from my hold on fatima marco myself.

A

A

In that case, I will thank again brian for one more time next time here. If we move.

C

A

Pleasure uh pleasure having you and thank you so much for being here. uh Thank you all for joining um again. If you happen to run out of ci cd minutes in the next days, there is a public now issue into the agenda, feel free to add a comment there with a screenshot of your usage, uh we're trying to address that uh right. Now again, we are wrapping up the hackathon by the end of this week.

A

Maybe we're going to give an extension for reviewing all the open, mrs because we have a few, mrs, that had to be like broken down or they had to be reworked, so um yeah we'll try to evaluate the situation and see by the end of the week um and yeah I'll, see you all again next week um with uh another topic. Hopefully another um gitlab, remember not that we don't love seeing you brian.

A

We do we're trying to engage more gitlab team members, more maintainers, um so yeah until then be safe, happy, healthy and I'll. See you all um online okay, bye.

D

Thank you thanks.