GitLab Code Review Weekly Workshop, 10 Feb 2023

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Code Review Weekly Workshop - Feb 10, 2023

Description

In this session we show & tell some lessons learned from Code Review and pair up on a review.

00:00 - Leaving things better than found, and non-blocking comments
08:06 - Finding existing utilities in the codebase
19:00 - When is there too much documentation?
32:05 - Pair review

A

All right, thanks for hopping on the code review, weekly Workshop um I've, just got a small thing that I thought was interesting and wanted to and encourage with code reviews that are seemingly really straightforward. There's things we can just bring up and talk about and I wanted to share an example of that. uh But if anyone else has anything they'd like to add to the agenda, this is a good time too, and then we can pair up on some Mr reviews.

A

I have some uh merge requests to reveal uh so I always enjoy getting my work done, while we're all on a call together, um but yeah I'll just share this one little example uh that I wanted to share with you all.

A

So this is an MR that is updating these.

A

um Some of these constants came from a feature or domain specific constant file, but we actually have this uh graphql shared constants file, where we put all these graphql type names.

A

So I guess this is kind of something that people have been curating uh while doing more. Graphql stuff is, let's just put all the type names in One glorious file and that's fine um and so we're removing some of the ones that were duplicated and having them reference. This file, um some of them, are also just like hard coded like here and so we're having them reference the constants.

A

um So it's really straightforward, uh but there's a couple of things uh that I observed looking over this, um probably one which let me start with this one. uh So what's tough with all this is it'd, be really nice if we, uh if there's an automated way to just completely verify yes, this is a pure refactor. None of the values have changed, but that gets really tough with when we reuse the same constants in the spec.

A

uh There's not really 100 guarantee that these values match. So when I review this I kind of have the two things side by side and I'm. Just referencing like hey, is the value of this one.

A

The same like I'm, just gonna, I'm able to verify this in five-ish minutes of like okay, we did the refactoring and all the values matched up, but I did find myself just thinking like man I wish our tests sometimes I wish we just hard-coded the values, because that's what we really care about and be nice when we do these kind of things, was it a pure refactor or not and I get the benefits of not doing that but I just brought it up.

A

So I was like Hey, anytime I'm thinking, even if it's like a large group, unowned thing of like I wish. We had this. That's a great thing to bring up in a code review and um had a.

A

um Just saying hey, maybe we should write some docs for it, but I think that this is probably something we need to just discuss. So I ended up just creating issue just to talk about it, um but it's a non-blocking thing, and if this is the only comment, I would have went ahead and just merged this. So, uh but because of that I'm going through this kind of refactoring line by line and I see something else where pre-existingly, that's not a word, but I'm gonna use it.

A

We were interpolating parts of a graphql type into another graphql type, and that's not great, because these graphql types are completely uncoupled and so like in the back end. This is referenced as CI project variable. It doesn't interpolate any values inside of it. So us doing that on the front end we're creating some coupling is not actually there.

A

um So, even though this is a pre-existing thing, I think hey. We touched it. Let's try to you know this very small thing. Let's just clean this up, while we're here and and Khan gets that, so that was that was helpful. I probably would have leaned more before it's. Let's do this in a follow-up if there had already been some Mr fatigue and like we had already gone back and forth on some things, but since this is just the only thing that I observed and then we're good, oh it's like okay. This is really small.

A

Let's, let's just take care of this and then we'll merge this, uh and so, um even though this is a pre-existing thing, it's like yeah. We want to clean up as much as we can um to leave things better than we found. It would be great. uh So that was the only somewhat blocking thing if Kong had said and uh like hey yeah, but this is a pre-existing thing, so I don't want to do it. um He wouldn't say it like that. uh I would be.

A

um I would be fine, but I would hope that we'd realize, but that we just engaged in a back and forth.

B

We could have just.

A

Done it, you know, so that's uh I, don't mind bringing up comments about pre-existing stuff, especially if it's small and especially, if there's not already a lot of back back and forth. uh The other thing is here's just a non-blocking question. So this is a hundred percent.

A

Something I was not anticipating as doing here, but this shared constants file, references, ee, specific types and I'm, just not sure how we feel about that or what we want to do about it, and so, rather than thinking about it and me doing a bunch of research on my own I'm, just asking the question we can just talk about it and uh it sounds like Kong was kind of anticipating. Maybe we needed to just move some stuff, uh but he is asking of what, if I've done a follow-up?

A

It's like 100, that's what we got to do so um after that we were good to go and- and that was it, but I did think it was just. This was a good example of I I like to just start non-blocking conversations when we come across code because code review isn't just an approval process, but it's also How We Do collaboration and brings up some good discussions. So yeah.

A

uh That was the example I always wanted to share. Does anybody else have any uh code review questions or things to show and tell.

C

I haven't seen a review for the longest period of time, so I'm just here to watch.

B

Oh man, all right.

D

um I could share one that I did recently where I just hold on. Let me see if I can find.

B

It um so I can blanket nope, that's not it.

B

Okay, hold on I, can't remember who it was.

B

uh I think this is the bridge request. I will share some screen. Let's see, let's share the one that y'all are on for me and then I'll pull it over there.

D

um So this was um assigned to me as a database review, which was always very stressful for me, but it's getting less stressful. Still, not my favorite thing to do. um But what I wanted to show was a conversation that we had.

B

D

I guess I could show what the Mr was doing. Yeah it seemed pretty simple: they were adding to a finder the ability to scope, the execution method to like specific IDs, um and so they added some filtering and, as part of that originally had added.

D

Let me see if I can get a previous version. Oh please, don't! Oh, no! Okay,.

B

D

Let me see I don't know, I did use that dip trick. Thank you Paul. It was.

A

D

Helpful it once I figured out how to run it properly in film with the right spacing from the last thing. It really really helped um one of the things that they added as part of that was a new scope with a where clause for IDs.

D

um This has since been removed due to the conversation we had, which is why I wanted to show it. um Someone asked a question. The previous reviewer asked the question: I'm, not sure if we should be using the scope or if a where Clause would work here and so I'm like well I, don't know I feel like we typically do, recommend using Scopes, um so I kind of just started. Looking around the code to see. Is there a method called for IDs or like something where there's a scope where we're scoping for IDs.

B

D

I learned something which also helped these two folks learn something. uh There is an application record class that I think all of our um models inherit from and I did double check, but it has all these Scopes that are already like there. I had no idea these existed and.

B

D

I thought that was pretty cool um or like it's a class method that is essentially a scope, but yeah I mean I knew about some of these, so I've used like a safe finder create buy that one's pretty popular um and I'm sure these are in the documentation.

D

With fast read statement, timeout I've used that one.

A

um When you say you're sure these are in the documentation and.

D

Are like developer guidelines, yeah I'm sure somewhere this.

A

D

I know is in there this with fast read statement, timeout, I, think, okay um and I know: there's mention of safe, binder, create buy I, don't know if there's like a specific page that covers all of these helpers, but I know at least some of these are in the in the docs.

A

I I think that's optimistic of you is.

B

B

D

Look Oh no you're wrong, you're right, but some of these are in there I know, yeah.

B

D

This one's in there at least this one one of these is in here because they do talk about uh especially the database reviews. Like you know, I open the guide, um so maybe yeah.

A

At the bottom, there yeah it's.

D

At the bottom, there's also one about plucking, IDs um I, don't know, there's definitely some. Maybe there could be more documentation around these.

D

B

D

Go look to find them, but um I thought it was neat. I mean I, I, often look I! Guess when I'm doing code reviews I am often going and trying to look and see what exists in their current code to learn like a is this something that's done often or B? Is there perhaps a different way to do something that might be better? That I, just don't know about yet yeah, so I do rely on I.

D

Do rely on the docs a lot, especially for the database reviews, because there's this huge list of things you're supposed to check and also a lot of like rules around the different migrations and how queries are supposed to work um I. Also for the life of me cannot remember how to read, explain plans so I go back and look at that a lot um but yeah. That's an interesting point like it's not really mentioned anywhere so I, don't know how you would like discover well.

A

That's what I'm curious about what? What did you search for that.

D

I I searched four four IDs didn't find anything. Then I just started looking for scopes in the code base to see like where Scopes were defined. If there were any Scopes that were like specifically.

A

B

D

I, just uh let me like I was interviewing mine, I, think I was like scope and then I, you know, put it in the models directory models.

D

I was like looking to see if there was any and I happened to look at that one. It was just by pure luck, probably um because I think we do do this a lot where they're like four projects for this other thing, but I wasn't sure if there was one where we actually had a scope like four IDs yeah.

A

That's pretty cool, like that seems really helpful. um Yeah good, find.

D

I, don't know how, like maybe we should open up a documentation page for this I, don't know where it would live, but I'm going to put down a little note for myself to look at maybe starting one.

A

It's it's well! You know it's hard, though, because I like I.

D

Don't know if people look at that stuff either but like when you're first.

A

Right I clearly was just I clearly revealed my where I stand on that Spectrum. Oh.

D

I really like documenting things, which is I, think uh um sometimes the strange Behavior.

A

Yeah I think it's good I. Think people look to the code first, because they're always looking for patterns to follow yeah.

D

Yep I definitely did look for the code first, because I was like okay I'm supposed to be doing the database.

D

I, don't know the answer, so I'm gonna go look and see what's in the code base which all it may not be right, even though it was in the code base, but at least I can see it like multiple times then I'm like well. Clearly, this has made it through multiple types of reviews and it's probably okay-ish yeah.

A

We just talked about this last week, though, is I mean we do get Legacy patterns and and anti-patterns can get sticky in our code base and spread that way, yeah and then it could be really confusing to contributors when and they finally hit a reviewer. That's like hey. We shouldn't introduce code that looks like this and they're confused, because it's.

D

Already in the like well so here, why was it okay over here so.

A

What we concluded- and this is really this- is really good. We should be more diligent than this documentation updates are great inline code comments would be really great, pointing to some sort of documentation or pointing to an issue of like hey. We don't really follow this pattern anymore, like don't copy this, uh but I almost wonder I, almost wonder if, um uh if we have like a utility and was like Hey, this utility is really nice.

A

Let's increase its usage I wonder if you know, are there a lot of places where we can replace some of the custom created Scopes and uh uh if we can replace some of the custom created Scopes and um create an epic to just like Hey we're going to migrate everything to this new utility, because we can do it and it's there and that's how you promote usage is actually increasing the usage.

A

um The documentation for utilities would be really great, I. Think it'd be. We talked about two on the front end like using JF stock to generate an actual, like you know like how Ruby has the cool function? Part like you, can what is it? What am I looking? What am I talking about? Oh.

D

Like the docs, where it's like, here's all the methods in.

A

Here, yeah, you can search for all the methods and see all their descriptions and stuff. We've talked about doing something like that on the front end, um so I'm like that'd, be so cool on the back end, yeah.

B

A

um So maybe even just commenting the function really well and then yeah. If we had something like that, that would be really nice yeah.

D

um The Epic idea is interesting for that um yeah I might look at that and see if I drop, that in the or.

A

E

D

Like maybe we don't need a Robocop, but like the way that we handle RoboCop violations would be done, the same way, which is like there's a long list of stuff, and then we invite everyone to contribute and then eventually, it kind of fixes itself.

A

Yeah uh um and if there's some people I don't know if anyone's generated a script to buy it for it, um but like yeah, you kind of want to create a bunch of issues for a bunch of different of the full. You can do it by folder. You can do it by file that finds the stuff, but I love doing this too, because it it creates a bunch of issues that could be easily picked up by The Wider Community yeah.

B

A

um Those are really cool efforts.

A

What's up what's up Michelle, where are you gonna watch.

C

I, like I, have a very orthogonal topic to this, like since uh you're talking about documentation updates.

C

um Let me preface this by saying: I, don't know Ruby and I've not worked on Ruby much so when I did get a chance to work on Ruby or one or two masks. I did end up, starting with the documentation and what I did notice is uh I closed it out quickly, because there was a lot of things to read uh right so now, I know what the benefit is of documenting it, because otherwise it just becomes tribal knowledge, and it just goes when the person is not available.

C

Yeah the reason why I asked that is because I just when you all were talking about- maybe you should document this it just kind of reminded me of when I looked at the documentation. I was like huh that's a lot to read: I I, don't want to I'll just give it yeah.

A

Yeah um I I do think you can get too much um because we.

C

Wanted but I don't know if it doesn't have any better way to do it. That.

A

I I think um I think that the I think you kind of have to be able to approach the guideline at levels if I'm at a really high level, but stay at that level, and, let's just say what we need to about the high level and then, if I'm, in here's very specific testing practices, I was like: okay, that's great I'm, just in testing land and I'm just seeing code examples.

A

These are a bunch of different testing practices uh for describing specific functions that does sound like we got. We've gotten really two, maybe detailed into our developer guideline where it'll be really nice. If we then had you know that kind of ruby-esque API documentation, uh yes Vishal, you are 100 correct, that is gonna, that's gonna, be a one day thing: no I'm, not I'm, not joking, like uh the amount of Open Source work, that's being done to to replicate chat. Gbt means hey. We can point our own information at it.

A

This is 100 gonna happen to the gitlab handbook, because I.

D

Mean I feel like. Would that then allow? uh What's? What's the feature that we have? That's like the coding Helper, because that could then be used to be like hey. This method exists, yeah.

B

D

It could be used here instead of creating the scope, because you know I know I I do agree, there's a line for documenting things too much I struggle. When people like write long paragraphs of things because then I'm like I, don't want to read that I don't have time or I don't have like sometimes I'm, not in a mental space where I'm like I can like read this and digest it. I need it in like smaller chunks, but it's like a balance I want to have between when I do a code review.

D

If I want to go search for something in the docs like it needs to have the right, it needs to be findable.

B

D

You don't need to be a lot, but it needs to be something and I wouldn't even know what to put in the docs for what I just showed, which is like. There are a lot of helper methods for database in this file like but I. Don't think that would be searchable where someone could find it during a code review, yeah.

A

Yeah and that's that's a really interesting point too, like and I think that's would be. The key question is anticipating someone introducing a duplicate? How do you find we already have this thing and or how can you do that in the quickest way possible and Terry you did? The best thing is like I'm just going to take a quick look and then you happen to run into it. Yeah yeah, not many people are gonna, engage in that quick look, not many people are going to if they did may not run into the example you found.

A

B

A

Definitely a problem that goes beyond just you know, a very small scope about filtering with IDs I. Think it's it's worth thinking about. How do we.

D

Or even finding like testing patterns, we just I just found I work on the global search team. I've just found a testing helper that was for written by someone on my team and they don't work here anymore. So I guess it's like it's almost like that knowledge that, like kind of left with them that this helper method exists and I'm like oh, this could help us do testing better yeah.

A

Yeah I well and that's a real that's a really good point. I do think and I'd love to hear what you all think: um Fishel Chad uh and Terry. Some of that like if I've just introduced a new thing that I think will be helpful.

A

We used to do this so much more in front than than I see us doing these days, um but we would announce it on slack and we would we would try to give some higher visibility to it, um because the document, the developer guidelines, is not a very high visibility place to communicate, um but we would no at whatever level it is. It's like. Oh, you know, there's a new property to create alert to do this thing now. Everyone just keep. We all use this thing so be aware of it.

A

If you're writing these things, I.

D

Think it's a good idea, something post on our Channel I can borrow my team and I was like I just found this method. Could you use this method to write these tests differently yeah and they were like yeah and I'm like oh okay? Well, first I wanted to ask them because they wrote These very like great tests that test a lot more deeply uh certain areas of the search code than we have for other areas, but I'm like oh. This might make it easier.

E

My recent experience was uh like the get lab flavored markdown specification and snapshot testing framework and I wrote a lot of docs on that. It's like uh I, intentionally kept it like one giant page, because I think that's easier to navigate as developer docs, but, um like I'm glad I did because now the team is splitting, that's going to be.

E

um You know the other team and um them taking it over and himanshu and I feel good. Like I'm handing over this knowledge, you know it's not all just in the code and it's like here's, my my vision of this and then also everything I didn't get done. I have a big checklist of an issue of the things that I wanted to do. It's not spread across issues, but it's the least captured my thinking at this point in time and also the way I publicized. That was I, really involve them in it.

E

I asked them for feedback. I was like. Do you think this is too complex? Are you okay, with this documentation? Can you review this doc uh updates along the way and then I also really publicized it by liberally in the emrs? In my code, I was like or in the description of the EMR I was like. This explains everything about this I'm not going to re-explain it here in you have another Mr.

E

You can read this Doc and specifically this section of the doc to explain all of the context here, because it's a lot and it's nothing to do with anything else anywhere in the app.

A

Yeah, yeah and I think that's that's 100 of the way to to do it because there's a lot of context to it and so I was going to like classify the kind of and I would like to hear what you think Chad like that, that kind of documentation that seems to be like this documentation gives context and vision to the thing right.

E

And it's literally like a goal section right: the goals of all of this right.

A

E

How we implemented it? Here's what all of these files look like. Here's, the scripts, here's what they do! I.

A

Mean yeah, it has implementation plan stuff in it too. It's like yeah that um that that, even just how different that code needed to be warranted, I think that documentation as well so I was like yeah. That was 100 the right way to do it and I'm not trying to say hey. We shouldn't document things, but what we do at gitlab it can get too much like we have so many utility functions. We can't document everyone, they're they're, not going to be findable, but I do really like the idea of.

A

Why aren't we publishing like API docs man, we've talked about this for years and when he actually put together this for us a while ago, but I think we've recently, we weren't really using it. So we we may have turned it off, but having a place where I can.

E

Yeah I feel like this is something that AI will do sooner than later.

A

Oh, it's gonna. You know.

E

Don't you want to use this helper here, like oh yeah, totally, do that.

A

Oh yeah, it's gonna be in like well we're doing a git lab of you know: hey, we document we're a doc. We try to document. First, we have this huge handbook uh and huge developer heads like so we do have a lot of documentation.

A

It's gonna be fantastic, because I'm actually gonna, we'll probably shift from documenting for readability and documenting just for the AI to learn everything that'd be insane, be insane yeah.

E

Okay, go ahead, Michelle.

C

No I'm just waiting for that, like I wish I I feel like there's already something we should have between like allow us to train on a specific given of data right. uh Isn't the code search I mean the incubation engineering person released something about uh uh the Eis trade.

A

Yeah so I think so they're doing uh I'm not sure what what models they're using, but there probably isn't like the Codex model, that's trained on a lot of okay.

C

It's already a trade model.

A

Programming stuff, so it's very programming, specific I'm.

E

Excited about how it's, based on too right.

A

And honest I I believe it is. But yes, we do have a code assistant, incubation engineering thing, that's I think that's already. They already have a integration with our vs code extension.

E

So I mean there's incubation engineering, but it's also a specific AI is just a specific category right and I. Think the uh the code assist information. Engineer may not be here anymore after.

B

A

E

But uh yeah Monday, I'm gonna sit on this panel for for new new programmers and like the topic is like, is AI gonna take our jobs? Is there any? Is there any point in trying to become a developer nowadays, so that should be fun? I'll have some opinions.

A

C

Is how how much I know about Ai and ml right I just said I feel like there's, so that's what something will be already between. It should be so easy.

E

It's like at the beginning, you wrote machine code right like it was barely a level above binary right. You were moving around memory and then there was you know there was languages like C and then higher level languages and then Frameworks like rails. It's like you're, writing, English, like higher level, abstractions and I. Think the next thing is like: okay, you, you need to be an engineer for your fronts, for AI to be able to effectively control the AI to do what you needed to do. It's just yet another higher level of abstraction which, when.

C

You put it like that. It sounds like yes, it is good, definitely going to happen.

D

Yeah, that's another framework right, you know it's like JavaScript and then like View, and then this is another thing. On top of that,.

A

I'm gonna be so sad when I'm, not writing code and I'm writing. Ai prompts and it's like. Oh I gotta tweak my a prompt, oh there's, a bug in your prompt. Let me debug it for you. I'll I will.

D

Be I'm gonna open up a bakery before that happens,.

A

It's a good idea: uh yeah! That's a.

B

A

um Yeah we got a couple minutes left. uh Do you all wanna uh hop on a.

B

Code, let's do some of your work come on.

A

Thank you all right, I love it. My.

E

Name is Francis, so fun.

A

um So let me see yeah. These should be good uh I'll go ahead and share my screen so that we can pick one um all right.

A

This is one I've already started. Reviews probably needs to just be merged, um so we can hop on it. This one is a customer's Mr, that's a private repo, so I don't know if I can do that on the recording.

A

um The uh this one is for your view, and it probably just needs to be merged as well. There's also a customer's one- and this is just a very straightforward web- ID- updating the vs code, Fork um so yeah. uh How about uh hopping on this one at the bottom of the list.

A

Okay, anytime so I've already I'm gonna. This is now uh cycle. Two of I left one round of comments, but I think it's gonna be ready to merge, but I always say I'm. Looking at this, whenever I start looking at it.

A

So I'm wanting to see what was my previous comment was yes, uh this spec was very um underwhelming, to say the least, um there's a whole there's a lot of uh testing app and we weren't introducing anything new. Here we were trying to just replace one component with another, but it's like okay, we're touching the spec now and this the unit test has a lot of missing coverage.

A

We should probably do it and while doing that, uh I um but I have up a gather patch uh to like here's, just how we could approach improving it, but while doing that discovered that um there's kind of a quirk with one of the prop one of the computers that we're setting, which affected a prop we're passing down. So um so, let's see the state of it now um and I will use the the diff of diffs tool. Let me move my.

A

B

Open up a new terminal, let's go to the bitmap project,.

A

Okay, um I'm gonna go ahead and when I left this comment, we were at um we're at this Shaw. But now we are at this one. So we'll use the diff of diff tool to see. What's going on uh so I'm gonna.

A

Pass into diff from Master to when I last looked at it. This is the original diff and then I'm going to curl the most recent death.

B

A

Yes, so this is new line, we're adding, but you see on the uh on this side.

A

um This is the little bit that we need to change to fix. Our computed was previously returning would return a string of undefined, which doesn't really make a whole lot of sense, and then this is all of the test. Yes, this is all the test updates that I was suggesting we'd make um and so yeah.

A

We just touched those two things so I'll now, actually I don't really enjoy reading diff of divs as much I kind of just get a high level understanding of what it actually changed here, uh but I'll go ahead and pick a quick look once more time, and then we should be good to go.

A

Let's discard everything here.

A

B

A

Here's our drop down we're using this fancy thing now. Guess: we've looked at this nothing's changed since I looked at that which is good to know nothing changed to that since I looked at it. This is the test suggestions, I, planned and I'm just going to take a quick look, uh uh we're rendering it doesn't emit anything selected and then, when we're already selected, show selected yeah all right, I think um because I have some context here: I don't need to re-review it from scratch.

A

I think we're good to go so uh I need to resolve threads.

B

All right here we go.

A

Thumbs up approving all right, we gotta put in a gif in here.

A

C

Like this I always wondered, you know how: how are you storing a juice in explosions.

C

A

Yeah I, you know every once in a while when I'm, really uh when I'm, really bored I would say honestly is when I'm really bored I'm like okay. Let me go find a new Chef to use here, but I have a folder of just that. These are my favorite ones that I'll that I'll use for these all the time, all right cool, let's get it going!

A

uh I I, remember, testing this too. So I have high confidence. This works out and I don't need to test it again. um Yeah all right, I'm before I need to start a pipeline before I can do.

D

You check that every time.

A

D

The edit commit message just.

A

D

I need to start doing that more often.

A

Yeah we kind of do need to because, like we got to watch for the change log, often I'll have to change the change log. If it's like.

D

On a later commit, like someone said, oh, you didn't add it, so they go and add it not. On the first commit or.

A

They at oftentimes it will have a change log, but it's behind a disabled feature flag, which means it shouldn't have a change log um or it has a change log and it says, like added when it's really should be fixed, like it's all the stuff. So it's like kind of I, don't know how clear it is, but this is somewhat of the maintaining responsibility. It's just to double check that squash. Commit message, needs to.

D

Or it is mentioned.

A

Let's check out the docs actually better.

D

Yet actually I guess I. Never uh all.

A

Right our get lab maintainers responsible for checking the squash. The mint message.

A

um I.E is the change log entry, correct.

C

I wonder if you can ask literally ask all get lost specific questions, but I think is gitlab.

C

Oh, come on. Okay,.

A

Yeah, it's it's it's just it's not.

A

It knows the answer. It's just it's. It knows the answer is holding back on me.

E

So that that was a pretty accurate answer, yeah.

D

It says like it is a responsibility of the author, but the maintainer should also check it, but ultimately is responsible of the author.

A

Yeah I wonder what our docs do say about it: I'm gonna, look at it real fast, but yeah. So this is a change yeah. This is all good yep, moeps, all right cool thanks, that's helpful um yeah! We got just one more minute, so let me do check our.

A

um Searching it for.

A

um Maintainer code review uh commit message.

D

It says you are encouraged to select to commit with more informative, commit message before merging.

B

But I don't see anything else that would.

A

When Mary Jane should only use if the authors.

D

Yeah and then like right at the bottom of your screen, is what I was um reading, which is from merge requests that have had it set um you're encouraged to select a commit with more informative, commit message.

B

Before merging.

A

Yeah I would say it's not uh it's not clear, but I I run into it all the time where the change log isn't right. We used to I and I wish I.

A

There is a lot that a lot of robustness that we lost by moving changelog to the commit message. Then we used to have the changelog files I used to be able to leave comments on the changelog of being able to say hey. This actually should be this and it didn't matter what the commit message was.

B

A

Don't we still I, don't think we still respect reverting uh if I, if we have a change log but then I've reverted that commit I, don't think our changelog generation will understand that I reverted the change. You know what I'm saying well.

E

It wouldn't be, it would be a removes change instead of an ads right.

A

Right but then what shows up? What shows up at the Docks that the or the the change.

B

A

Added the feature we removed the feature, I think it would be I guess.

E

Yeah I mean if they made it into Master, that's the appropriate thing like it was there and now it's not that's.

A

E

D

A revert Mr supposed to have a different change log than.

E

A

No, like it's just a.

E

Teacher it should, if it's removing a feature, it should have an appropriate one that oh.

D

Okay, so it's different than a revert: okay, well,.

E

It could be a revert, but you should amend the revert message to have the correct change. Log entry, which would be removes.

A

E

If it is removing some previously delivered yeah, but.

A

It could be interesting because the game would be rather than adding, which clearly has the opposite of remove. Sometimes it's just changed.

B

A

Fixed and it's like well, the fix actually didn't work. So it's it's not clear on fixes.

A

B

A

Right, anyways thanks everybody I'll catch you all later bye.