🚀 Space Reviewers 👾 Episode 2 - Django - Include ModelForm and its Meta options in reference docs

Hi

Speaker 1: everyone and thank you for joining us. We are Rafaela in uh in team here and And welcome to Space Reviewers. Tim and I have different experiences, but neither of us is an expert with the Django codebase. So the goal of space reviewers is to show that everyone is capable of a beneficial code review regardless of their experience. So this is our first our second episode and uh for those who didn't follow us in the previous one.

Speaker 1: We are going to share some quick explanation on how this library works. We have just chosen the PR. And then we are going to review uh live with uh with everyone. Um And we choose the PR based on what we are more uh um more familiar with, for example. Okay, why uh uh this online interview is um meant to uh prove that everybody are can

Speaker 1: review our PR so we are going to learn together and with the code review and also uh um sharing some experience and some our um our experience with the Django code base.

Speaker 2: Yeah. So the

Speaker 1: Yep.

Speaker 2: I was just gonna say the to suggest to PR that there's this link over here, just because I can highlight it. Um the Jango Not the the GitHub Jangonaut Space Space Reviewers. That's where you can go. And then there's a link to submit a form to suggest a PR to review. And then we also have a link to request to join the Zoom call. and attend, you know, and be able to chat with us live because we're not monitoring the YouTube chat.

Speaker 1: I think every everything will be on our uh repository.

Speaker 2: Yeah. This page.

Speaker 1: Over here, yeah.

Speaker 2: And then yeah, to our current one guest, uh yes, we are operating under the Django Django Not Space Code of Conduct. Um, please be respectful. Uh yeah. Alright, we're up to two. Nice. Okay. Um so we are at episode two. Uh if you Go to our repository, episode two, and then ticket 35831. That'll bring you to this page. We have some summarized information here. that effectively goes over the track ticket. Um so this thankfully this one is actually fairly short, but what are we

Speaker 2: seven comments? Well it's not even seven. Five comments. And then we also have a link to the actual PR. So the issue that we're looking at is include model form and its meta options in the reference docs or in reference docs. From Some of that documentation for model form does already exist, but it appears to be in topic format versus the reporter here is looking for it to be more of reference documentation. And so there are different types of documentation. And so I have this link here.

Speaker 2: We have this link about Dio taxis. And this is how Django's documentation is oriented, where there are tutorials, how-to guides, explanation, and reference documents. I believe the explanation is topics for Django. So that is what they mean by some of this documentation exists as a topic. They would want it to also exist as reference documentation. Um, so if you're ever curious about how Django is actually architected its documentation, this website is actually a good place to go. Um is that is that a fairly good summary, Raffaello?

Speaker 1: And it wasn't not so long, so I saw lots of comment inside uh uh GitHub instead Um but because I I think I I have some trouble with uh because of uh the the English things I if it's okay for you, I try to make us uh what I uh I summarized for uh for what they uh they talk about uh and you let me know if I if I'm just just wrong or I I misinterpret something.

Speaker 2: Yeah, for sure.

Speaker 1: Um At first uh I saw that uh they uh they discussed about uh where the uh this kind of documentation should have been uh um posted so um they decide uh specific refs and under for uh the form set And um another interesting things that I noticed uh uh is about uh um How uh to share or not to share this API because it could be uh also private or not not public because if it's gonna be public

Speaker 1: uh it has to be maintained and good uh uh and have a good explanation and also have uh some um I don't know the the pattern that uh every developer can can follow. So uh it you have to rely on this this new API. And uh the last things I I noticed, I think, um is the concern about uh what uh uh If s uh if an attribute uh uh is not uh uh inside uh i uh you um you mention an attribute that is not uh uh mm is not existing, what should we

Speaker 1: uh we be done? What should have be done? And I saw that there's um a discussion about that about up mm abdom abdomination

Speaker 2: Oh yeah, uh admonition, this word.

Speaker 1: Admonition, sorry.

Speaker 2: Oh no, yeah, I I rarely ever use that word in my day-to-day Okay. Yeah, I have not gotten this far yet. Um I I've glanced over it. I haven't read through the other reviews from uh actually the review from Cliff. Um so Um okay. That was a really good catch on documenting this would indicate it's public or private. Um I don't know if I had thought of that. Yeah. I I don't I wouldn't put that up.

Speaker 2: I I think it's worth making sure that it's raised. Um I don't think either you or I are really in a place to make that to say like hey this should be public or this should be or like this is private because it's still in flux. Like we we don't work with it on a day-to-day basis. So I think the best we can do is, like you said, I'd bring it up for other core like contributors, regular contributors, to then make that determination. Yeah. Okay. I don't know if you had I think your summarization was solid. I I don't know if there was any questions in it. Um I think I might like to take a little bit of time here to read through the docs

Speaker 2: that Jern Werber has added. Yeah. What do you think about taking uh like five minutes to to read over them

Speaker 1: Do you mean the the file that um that have been changed?

Speaker 2: Yeah, yeah, to just go through the the updated models. txt these these two files Yeah, I'm gonna pull them up. Uh Easier to read it. Okay, so that's not even a real big change

Speaker 2: This has taken me a little bit longer to get through than I expected. So maybe it's helpful. So I'm spotting a couple things here. Like some of the the documentation is Perhaps a little too much of a summary for what I would expect to see in reference docs.

Speaker 2: I'm curious like Maybe we can go through like what your feedback is and then perhaps uh We can spot some patterns and then if we do have feedback on that, if we want to submit that, um maybe we can review it that way rather than identifying every single point that we think falls into this pattern

Speaker 1: What what do you mean uh you do you expect uh more uh speci more specific

Speaker 2: This one here talking about the field classes of Maybe I'm just misunderstanding it's I I guess I've never you maybe I am wrong on this, but um so the reference docs here of So you can replace the field class. And it's a it talks about how replacing a char field with an email field will work Since the email field is a subclass of it, but then it indicates that replacing a char field with an integer field will not work because of the max length argument. And I don't know. Is that really the route that we want to take? Uh or is that

Speaker 2: the level of detail that should belong in this the documentation. Um and maybe in that case it is fine. But like even yeah anyway that's that's where I was having a little bit of pause

Speaker 1: Are you uh you just prefer uh um the preference related with the chart field uh instead of um uh showing what uh it will it will not work uh maybe just um write uh how uh you're supposed to uh use it instead

Speaker 2: Hmm. It's a good question. How would I Yeah, I spoke hmm I guess it does kind of make sense in that way of identifying like, hey, the arguments need to match. Um Error message isn't super helpful either. Okay. The

Speaker 1: type error gaming.

Speaker 2: Yeah, because I mean I ideally if you're replacing that field class and you use something that's incompatible The error message should tell you, hey, the field class you specified is incompatible with this model field. Um maybe it's because like the base types are different. versus this error message is what you're gonna get, you know, field init got an unexpected keyword argument max length to a beginner, that's pretty um obtuse or confusing. Uh so I guess if that's that's the way Django works, if we put this in the documentation, it is easier to Find that, you know, searching for it.

Speaker 2: If you run into this error, you can search for it and you can find the documentation. Yeah, okay. I'm coming around on that one Um, do you want to go through some of yours? Any questions or comments that you have?

Speaker 1: I think I um I think as mentioned in uh GitHub, I uh There's this question related with uh um how uh and what uh uh it's gonna be happened uh if you are try to uh to make an uh a different attribute that the uh the the meta expected

Speaker 2: This this admonition of troubleshooting the meta class issues.

Speaker 1: Yeah, because now uh so there's invalid meta class attributes. There's um Do you think it's m maybe it's uh just um just good enough and maybe make an um make a warning related with uh uh um how many attributes you can use and if you are not using one of these list attributes uh it's not going to to work So you have to uh look inside the documentation to understand uh uh how to use the these these kind of attributes. Maybe it's maybe it's a not

Speaker 2: So are you So is your question Do we need the I'm having a tough time I I think I might be understanding you based on like what my concern is. Um, but I'm concerned that I I wonder if I'm not arriving at the same distinction. Is it that Why why are we telling people like if you misspell something or you specify an attribute that doesn't exist that it's not going to work as expected?

Speaker 1: Yeah, maybe you're you're going to use the um um an attribute um uh for example I don't know maybe name name uh and okay it the it's not uh in the list so it's not gonna work uh and if the um if you don't receive any any feedback that it's not working maybe you just assume that it's working in in the right way I I I don't know but uh uh uh there's uh an um invalid metaclass attribute uh field where um where says that um

Speaker 1: only uh only this list of attributes um is going to work so you have just been born and I I'm not sure. Maybe it's enough.

Speaker 2: I think that's yeah, that's way more sufficient. I don't think it's

Speaker 1: Okay.

Speaker 2: Yeah, I would agree then. If we have the warning, so like Sarah uh agreed with us that trying to list off everything that, you know. Is wrong or that that isn't a thing isn't correct. But like if we are catching the case of these attributes are being specified and they're not being used I don't think we have to document that. Like that. Because that's a debugging thing where, oh, I set this code or I wrote some code and it's not working. But if the code actually errors out and tells you exactly what's wrong, to me that that's helping you there. Like the documentation shouldn't necessarily do the debugging for you unless the code doesn't work.

Speaker 2: So like that's it's the opposite of what we were just talking about. Um where that error message with the field classes, in my opinion, being insufficient. So I think the documentation can make up for it. But in this case, the code is providing enough information that the documentation shouldn't then try to also s you know, complement it. Like it doesn't have to. Oh, but that's for model options. We don't have that for here. Yeah. I I agree with this statement here uh from journ

Speaker 2: um from the author of removing this note and creating a ticket to Copy that meta this logic.

Speaker 1: Okay.

Speaker 2: What do you think?

Speaker 1: Okay, so basically he's going to uh um to make another PR to uh to raise a type error.

Speaker 2: Yeah,

Speaker 1: okay. I think I also noticed the fact that a it was uh related this this meta class it was also related with the meta uh uh the with um model of uh object let me know if i see

Speaker 2: That would make sense because it's it's being written for the model form, which will have some coupling to the model, and it's gonna probably use some of that. Um some of the same attributes on those the meta classes. Um, anything else that you want to talk about?

Speaker 1: um a consideration because I uh at first I didn't uh uh notice that uh meta it was not uh um Written inside the documentation because I was expecting to uh I remember to see some example, but I didn't aware that There's no there were no space for meta uh for uh uh model model um uh I think it was it what is it was interesting to discover this uh these this these things that it

Speaker 1: it was not um It was not available uh um separate from them the model object.

Speaker 2: Yeah. And this gets in how familiar are you with metaprogramming? Like the the actual concept of metaprogramming.

Speaker 1: Uh I use it uh for uh for admin in uh one of my projects.

Speaker 2: Then you're more familiar with it than I am because I I only know of it because I know Django uses it, but beyond that, um, yeah. So, okay. Uh What can you correct me if I'm wrong on metaprogramming being you're writing code to define other code that you can use? So Like when you define a model, it's not necessarily you're not actually writing the code that you end up using as a model. Or maybe forms are probably a better way. Anyway, when you define a model in a Django app. that model isn't used directly. Like you're defining the structure of it and then Django is going to create a different class for your model

Speaker 2: that you then use when you fetch it from the database. So like it's your writing code to generate other code. Is that close?

Speaker 1: Yeah, okay, I'm following it.

Speaker 2: No, no, I was asking you. Does that match what your understanding of metaprogramming is?

Speaker 1: Yeah, i i uh with meta you can just um um make more uh uh adjustable related with your uh with your needs something related with your your model or your um your class that uh has been nested inside meta

Speaker 2: Okay. Um I think we might have a miscommunication.

Speaker 1: Okay.

Speaker 2: There's um Like the term actual m metaprogramming. Um Yeah. I Okay. I think we're talking about it from two different angles here.

Speaker 1: Oh gosh.

Speaker 2: We're all right. We don't need to dive into what the actual definition is. Uh so I pulled up Um where is this? Yeah. Okay. So here's the code. Um and we were talking about sorry, I don't know where I was going with that. Um Excuse me. There was a comment some of the documentation I I there's one bit where At the beginning, there's a mention of

Speaker 2: okay, so here this other than at her Model form options, model fields, exclude. It's trying to indicate like one of those must be specified, and then the other one is optional. And then it also indicates it again. on the exclude of saying like one of fields and exclude must be set. I like how this one's written. Of one of them must be set. And the one up here, this to me feels. . . Confusing. Um let me see if I can pull it up in the code editor.

Speaker 2: Yeah, so this line to me is confusing of saying other than model and at least one of fields and exclude, meta attributes are optional. I think this might be better as um, how would you write that? Something like uh Model is required as well as one of fields. Yeah.

Speaker 2: I think this might be a little is a little bit more understandable. Um the other thing I was wondering is like, do we have other documentation that's similar to this, like on model or the at the model admin where we can replicate that documentation to make it easier. One for like translators and then also two like other people just reading the documentation, they they see this familiarity in like, oh, this definition is the same as this other spot. It all works the same.

Speaker 1: So the first the first sentence do you mean?

Speaker 2: Yeah, so I think these two sentences here are equivalent. It's trying to s tell you that when you define That you need to specify the model on your model form and then you can specify either fields or exclude One of these two must be specified and then all the other at attributes you can set are optional. And I I find it written this way a little confusing. Um

Speaker 2: because it requires you to like hold okay, it says other than model, which means all right, everything but model, and then it says at least one of these two. So now you have like this optionality, and then it's like they're all optional, which then means everything before that means they're required. And like that having to revert things around is I I feel like that's not written for understand like the ease of understanding. I think specifying them outright of These things are going to be required, this case, and then the rest are this other property.

Speaker 1: Yeah, because I I um I understand the sentence because I translated it in the entire sentence. it more uh it was more uh uh easy to understand when I translated it. Uh but now when I read with this uh your your sentence it's more uh uh more easy to read uh even uh even if if it's plain English

Speaker 2: Yeah. I think I I think it's a good thing to mention for sure. Um so I don't want to remove it there. What do you think about let me go ahead and undo this. Where is that? When you're reading the documentation here for exclude, it's telling you that I wonder if we should build this. To look at it. Um any sorry. Uh go ahead.

Speaker 1: Sphinx.

Speaker 2: Yes, yeah. I've Mm, okay. Let's see if we can do that quick.

Speaker 1: If you need it I think I made uh the the the the the This the solution I just copied inside my Django uh Django Django Django um copy what uh the the changes where he made it inside uh and I I don't know if it's work what i it what is going work best uh because I just copy and paste the the the Documentation that I made.

Speaker 2: Yeah, I think so. The instructions on how to make this. And I think I've already, yeah, I've already activated my virtual environment. Oh, this is gonna take a minute though.

Speaker 2: Whoops. Apparently trying to build the documentation. Um not a good idea. All right. Let me share my screen Oh, David says the docs is on the link for the PR the job, so I don't have to do that. That's great. I didn't know that. So how do we Let's pull that over.

Speaker 2: Oh, right there. Okay. So

Speaker 1: okay.

Speaker 2: Yeah, when it runs, it generates Oh look at that. That's pretty cool. Thank you, David. Forms for models, this is where we're going. Okay. I think this is where No, that might not be it.

Speaker 1: Firm refs ref firms model, yeah

Speaker 2: Well, I know there's uh Nope, that's not gonna link to it.

Speaker 1: The model form is it the little one?

Speaker 2: Ref forms models. All right. I am lost. Here we go. Alright. Never navigated the documentation that way before. So it's a bunch of new stuff for me. Well this is cool. I didn't have to almost break my computer. Um

Speaker 2: Okay. Looking at it this way, I do kind of like this of the exclude telling you either fields or exclude must be set. That's nice. I think I might need to read some more of Django's other reference documentation to know Is this common of saying like if this isn't happening, you're gonna see this type of error? Kind of like what we talked about before, of getting an unexpected keyword argument. Um

Speaker 2: Yeah, I feel like some of the documentation is written, not some. Documentation is written where you try to be consistent and so Me coming in with my own styles and tastes probably isn't helpful to like tell them, hey, I don't know if this should be written this way or if we should be including this level of detail. Um does that make sense?

Speaker 1: So you want to stay um um consistent with uh

Speaker 2: Yeah, yeah, because if other areas of the documentation excuse me Do indicate like, hey, if you don't have fields or exclude set, um like on I wonder if there's something in the model admin class where it's similar. Yeah, one thing I there was a I do like this and the error messages

Speaker 2: where it tells the user when there's an error, what that flow is. Like if it doesn't exist here, where is it gonna go to next? Um and while IDEs make it a little bit easier to traverse like how your inheritance tree given that if you're just reading the documentation and not looking at the code or like maybe you're not super familiar with Python or your environment isn't set up to do that, having this heads up of where these other error messages might be coming from is is really helpful. So I like that it calls that out.

Speaker 1: When you mention the meta uh the model meta option, there are options available, but it in as far as I understand uh it's it's really summarized, for example, uh option not that extra extraction um and then um if it's true this model will be extract restract base and I if if it's false didn't didn't cover the So I I'm not sure yeah, I I'm going to I I was just looking at the right

Speaker 1: documentation for for uh for model. I don't know if it's also uh beneficial to make some Um some comparisons for model and model forms.

Speaker 2: Let me So which page the Mata Met this one?

Speaker 1: The model meta options. I think I found it on um etiquette

Speaker 2: You're talking about this page, the reference docs for okay

Speaker 1: Do you do you think it's it's worth to to understand how this piece of documentation is working to um to understand if the meta uh the the the model form is H is m should we follow the same pattern

Speaker 2: I think generally yes. I think because this is existing documentation that is doing something similar that we can Assume that this is what Django wants, um and we can use it as like a definition for, but there are gonna be some differences, and I think that's that's where things are gonna diff uh yeah, they're gonna change or be different. So for example, like with the error messages and like the the the labels and stuff, everything here uh labels on the form

Speaker 2: fall back to the the verbose name on the the model And since the model is like your source of truth, it doesn't have that. So like we there are going to be some exceptions. Um Okay. So there is some code definition. This does give me a good uh another idea of what about the admin?

Speaker 2: And I'm trying to see if there's anything here. Just quickly scrolling through of um talking about errors or misconfigurations.

Speaker 2: Okay, so there is a bit of Um

Speaker 1: similarity.

Speaker 2: Yeah, yeah, some what is it called? Precedence. There we go. That's the word I was looking for. Alright.

Speaker 1: What are you looking for?

Speaker 2: So I I just saw this expects a dictionary, expects a Tuple uh the the expects phrasing. I was curious if that's similar. Um Okay, so it it looks like we the Language that Django's documentation typically uses is set whatever that attribute is to something else to do something. Um so I think that's probably something we could provide feedback on of set

Speaker 2: help text to a dictionary. Uh yeah, so like right here. mapping field names to whatever else. So that's yeah, I think that's something we can do.

Speaker 1: What do you think about reacquired inside the model attribute? Because at the first uh uh sentence that you mentioned about uh it was um Maybe s maybe trivial to understand one uh in this sentence. It's um it's really uh It's really straight. It's it's really obvious that you need model.

Speaker 2: Yeah. I think I don't know if it hurts anything. I would also wonder Yeah, I'm just thinking of like when someone uses reference documentation, are they I guess yeah, you're coming here for a very specific thing. Like if you're looking at it doesn't seem likely that someone's going to just read the reference documentation like a dictionary. Like you're you're using a dictionary to go look up a single word to see how to use it or like what um the definition is. So I I think if we look at it that way, keeping required here would make sense. Yeah.

Speaker 2: This is interesting. The the model form options I think this might be a little Sorry, if before I move on, do you what do you do you agree that we should keep required here?

Speaker 1: Yeah, yeah, I I think uh I like it most because it it's more uh understandable when you uh searching for the model uh attribute you see clearly that it's required instead of the first sentence that you mentioned that um the order than model uh and at least the one of fields or exclude meta attributes are optional

Speaker 2: Yeah.

Speaker 1: The first one the first sentence.

Speaker 2: Yeah, this one.

Speaker 1: Yeah, no, it

Speaker 2: has the same thing here too. Um Um okay. Um we're in agreement there. One um this Now that I'm looking at this documentation compared to the model admin, I notice that the the section or the field titles here have the class that it's set on and then the field name. But here it has model form options. I've been working with Django for 10 years. I've never seen this class before in my life. I don't know if a developer looking at Django would also know what model form options is. Like, does this

Speaker 2: provide any additional help or context. Like it if you're looking at this not to understand Django's code, but how to use a model form. I don't think this helps. I think this actually hurts because you're gonna look at it. I would look at this and be wondering, am I in the right place?

Speaker 1: I think it's related with the with the model first um the title uh at the very top. Yeah, you know where you are because the class uh model firm. Otherwise uh I don't think I don't understand if you are just saying that if you are not you are not going to search for model farm options. Specifically.

Speaker 2: Um, so I'm saying that if I'm searching for error messages and I find this link, and so this is where I come to Knowing like all right, this should be something in model form, but then it's saying error messages and it's providing this model form options is where you're supposed to set error messages, but I'm working with uh the model form and I have the meta, I don't see model form options anywhere. So like I I wouldn't it's not gonna help connect any dots for me. And I think it would be yeah I I think it there's a reasonable chance that a newer Django developer would look at this and be trying to figure out where they needed to find model form options.

Speaker 2: What

Speaker 1: what the best solution uh could be for you?

Speaker 2: Um I think removing it or replacing it with meta meta messages Um, I think those are probably one of the my initial two thoughts. Um Because that's how model admin works here, where you don't actually set it on it is something from model admin. But you're not setting it on model admin. You're setting it on your definition of model admin. So I Yeah. I wonder let's see if there's anything else in here.

Speaker 2: Okay. Alright, I'm wrong again. So looking at the model meta model meta options, we do define

Speaker 1: Yeah,

Speaker 2: is it a sign? Okay. Alright. Then I retract that comment. Batiste the delay. I gotcha. You could have saved me. He uh on YouTube he said using the m name model options is in line with the docs for m model. meta Not a hundred percent uh sure I agree with it, but I I'm okay with consistency. Um

Speaker 2: Is there anything else that you spotted that you think we should bring up or comment on?

Speaker 1: I don't think so because I um I see that they are they were me. They were made um lots of updates during the way. For example, uh removing some things, uh just moving the documentation to the related place. Also yeah, also I noticed that um for example my first uh um my first desire when I look into the documentation is just for um

Speaker 1: see uh the most important uh uh fields or uh in this case attributes at the top but uh I noticed that uh it's uh more consistent to um to have a list uh alphabetically ordered. So I yeah I I just I I will like to to see what's it's required uh first, for example the model one. But uh this uh uh can can break the consistency so this was my first idea to to see what what most important at first uh and then see to but that that's not a good idea.

Speaker 2: Yeah it I run into this a lot with API documentation. And in that case, a lot of times with API docs, it does Like the reference documentation is your introduction to it. And so it does indicate like these it starts off with the most important things. Uh if you have to specify an API key in the body or like the ID of something, it'll list those things off and then all the optional things will be under it alphabetically. And I kinda had the same thought uh when you were talking about excuse me, the required Notation here for model of maybe we should have a required section, but then where I got stuck was on

Speaker 2: um ah the fields and exclude where it's now one of these needs to be set and then that kind of broke my brain of How do I how would you organize that then? Like do you put both of them in the required section, but only one of them actually is? Um so all that to say, like I agree with you. I think Being consistent here is helpful.

Speaker 1: Yeah.

Speaker 2: I do have another thing I wanted to talk about is the title of it. So this file used to be for model form functions, and now it's changing to model forms. And I was first looking at it like a couple days ago, like this had crossed my mind, but I wasn't sure how to Reorganize it. Like I don't think this is quite how we have this organized is quite right. But now looking at the models reference documentation, I think maybe we should match this where we have model form meta options and then we have another file called um model form functions and keep this Keep the rename the existing models.

Speaker 2: txt to functions or model functions, I don't know, something, and then create a new one. called models. txt which will then be uh this model form meta options Do I explain that well?

Speaker 1: So basically you want to uh to to split the um this this um uh this piece uh of um of text of uh this piece of documentation on text and

Speaker 2: Yes, yes, exactly. Split it up. This would be model form options. Actually don't know if that's it right.

Speaker 1: I I'm not sure if uh Cliff and John Werber talked about this um this mm because I I remember they talk about uh the uh the position of the of of the decom this this type of documentation. But I'm not sure if they 're I think that the On October twenty three.

Speaker 2: Um , to twenty three Do you have a phrase I can search on?

Speaker 1: Hmm. And

Speaker 2: Oh, maybe right here, this one.

Speaker 1: Uh Ye yeah, I think also maybe yeah, this this one, yeah, this one

Speaker 2: Oh yeah. So he Jern had created a new file originally and then read Okay. Oh, I really don't want to be the person that comes in and Well, I I do think they should be separate files. Um they are talking about different things and we do kind of have uh some prior like it eventually the forms area should get into this which I think doesn't it already? Yeah forms is here Oh, oh, so we do have that already. It shouldn't be mm

Speaker 2: Okay. Maybe I'm just being too picky here. Okay, never mind. I think that's fine. From the yeah, looking at how it all gets rendered, it does clean up nicely How does this work for models then? Okay. Yeah, I guess that wouldn't I guess this is kind of what we're stuck with.

Speaker 2: Okay. Well, I don't really I think the only The thing I found that we can actually suggest that should be changed would be the um the phrasing on the fields of to match how it works with the the model admin. Um I seems like Cliff and Jern were really thorough with their early review and looking at everything and Um yeah, I don't know. What are your thoughts?

Speaker 1: Um you were talking about the um the phrase uh order the model and at least one of the fields were executed.

Speaker 2: Um Yeah, I'd I think I'm circling back on that one. I I um I think I'm good with that being in both places. Oh right, sorry, no, I I know what you're talking about. Um Yeah, where where did I put that? Yeah, this comment. Thank you.

Speaker 2: Okay. Anything else? Oh, this is Pi charm. Okay. Um all right. So we rendered it. Well, we didn't render it, but it this would be the other thing too of Do we at a minimum like this should probably use camel uh what is it? Pascal case? No. Snake case. I think that's what it is. Um

Speaker 2: So

Speaker 1: that's suggest snake case.

Speaker 2: Snake case, yes. Yeah. Oh, this is not what I want to search for. Okay, so we have I'm doing a quick search here in the docs for dot models import to see what are some of the other usages. And looks like my app is.

Speaker 1: Okay.

Speaker 2: Oh, there's one that does my underscore app. So like we're not even that consistent either. Um So maybe we just do a soft suggestion I've seen um some people provide this type of feedback. Uh sort of they'll prefix it with nit and say Sorry, like they're acknowledging like hey, this is a nitpick. Like I understand like this this is something small. Usually it means like you can ignore it if you want to. Um And so like this is typically

Speaker 2: I app or And like that's what the comment would be. Um I don't know Rafaela, did you see the the comments on Mastodon about some folks saying the the code review process for Django is a little too uh overbearing

Speaker 1: I remember the the conversation

Speaker 2: yeah what what were your thoughts on that

Speaker 1: I personally don't think it's uh in uh could be the case in this because uh um I don't think it's a really good uh um trivial problem. Uh I don't know if if uh Actually I I don't know why I I just don't um don't notice the um the import Um I just assume it was um it was okay but uh for consistency for consistency maybe uh I if they don't agree, I think they can they just can

Speaker 1: um um just don't make the change.

Speaker 2: Yeah.

Speaker 1: It's not a a really really um a really good problem. Um that it's something that if you if if the Uh if it's not going to to to change is going to um uh to to block the PR I don't I don't see this. But what do you think?

Speaker 2: Yeah, I I struggle with it. Um mainly because I think consistency within the Django framework and especially the documentation is important. I think having consistent language is also important for translations, because I imagine slightly different changes in English then can be translated multiple different ways. And so by not doing something and this is more I I I'm Taking this discussion of my library app, I agree with you. Uh I think it's worth us mentioning this.

Speaker 2: at least to say like hey we shouldn't be using uh camel case here like we should be using snake case or all lower case um to be more consistent. But then also like extending this discussion to what we were talking about earlier of the the labels having document um where the author has written it of expects a dictionary and then I'm going to make this adjustment of using you this should be set field name to a dictionary. That is Yeah. I like I get on on Mastodon people were saying like that

Speaker 2: if you're gonna make those types of comments like you should just make the change because Why are you telling somebody else to write the thing exactly how you want it to be? And I get that. Like that um I'm trying to do a little bit better job in my personal life of when I do see those type of things, like just go make the change, push the commit, or use GitHub's um suggestion. I don't know if you've ever used that this add a suggestion, which then you can change this to my app. And that's how you get this nice little thing. And then inside GitHub, they can actually apply those suggestions. So they don't If they agree with you, like it's then as easy as a click of a button. You don't have to go change anything.

Speaker 2: So I think if we're making it easy that way, I'm a little bit more on board. But it's still it can be a lot. Cause how many there are nine cases of expects A Well, maybe not that one. Nope, this one too. So if we leave a comment for all of these, this person's now going to get a code review that says nine comments, and they're going to have to address all of them. And that can wear people down. And so like that's yeah, the the problem. You know, we want this consistency, but when somebody doesn't match it You know, what do we value?

Speaker 2: The consistency or that contributor's state of mind, or like emotional state, or like making sure that they're feeling welcomed in the community. And I don't know. I don't have a good answer on that one.

Speaker 1: Uh if I could I think uh this uh uh the the the conversation you mentioned uh you mentioned on mustone it maybe it's more related with uh um more big uh I I'm not sure uh if that it was the case but as as mm as I can feel It's more related with um uh really good uh um big big uh work um with a PR and the Mm make it uh um make the change uh um um um not not an option uh I think. I'm I'm not sure if uh this is uh the what

Speaker 1: it's what it was going to uh what it was uh um worried about because I as far as I as as I um as I understand uh I I see this person that it was struggling with uh some review because um I I think uh um he wasn't able to decide what which one he uh he can approve or not. Because uh I I I'm not sure but because there there's no um um a clear example of what

Speaker 1: he mentioned it. But as as far as I understand his motion I think it's because of because of this, because someone made uh a a big work, a long work, and then um um uh he didn't have the to decide to he didn't have the option to to decline the changes but

Speaker 2: I don't know. That's a good point. Um Maybe when we specify this, it's hey, we noticed this. We're not sure if this is how um if this all should match. But we feel that like there should there there would be some value in matching this because then translators coming in later it's the flow matches. So then like maybe translations gets a little bit easier. And so like I think if we phrase it that way of this change isn't just for to match some sort of grammar thing. It's also like, hey, this could actually make things easier for other people. And I think if we phrase it that way This particular case is easy to handle.

Speaker 2: Um, but like you said, maybe this isn't what uh that conversation this PR and specifically isn't what that conversation on Mastodon had in mind, but I think it's important. Um I feel like it's important to me to make sure that I'm before I hit that review changes and submit review button that do a little bit of self-reflection on like is this review something productive is it gonna you know make sure is it gonna help that person feel welcome um yeah but

Speaker 1: I think that it's also um a problem uh for uh for every uh um every type of mentors, don't you think?

Speaker 2: I'd I'd I'd like to think so. Um yeah. I think some people uh Hearing enough scary stories about open source, I don't think it's um What do you want to call it? Ubiquito ubiquitous? Uh that it's not a constant. Like everyone, I think you'll find varying amounts of people being abrasive or mean. Means probably not the right word. Just burnt out probably. Anyway, I think we could probably Get close to wrap this up. Uh we can leave some comments. Um

Speaker 2: Yeah. Do you would you hm

Speaker 1: okay?

Speaker 2: Which one would you prefer to leave? So we have We have uh agreeing with Duran Werber on opening up a ticket for an invalid attribute uh or attribute set and then suggesting to follow the model. prepopulated fields pattern. And I think. Yeah. And then the last one would be Restructuring um some of these document Oh then we have the the my library app.

Speaker 1: Okay, basically we have three I don't know.

Speaker 2: I think it's three requests for changes and then one comment that we agree with where Jern Werber landed So I I I think I'd like to do leave this one if that's all right. This model is required. Um the one about Other than model and at least one of I I'd like to leave that comment.

Speaker 1: Okay.

Speaker 2: Okay. Um Well, I definitely just clicked the wrong button. Oh well. Whoops. Well, that one just went live.

Speaker 2: Uh Okay. Uh did you what did you leave either of these?

Speaker 1: I'm going to leave the agreement on opening a new ticket for invalid attributes.

Speaker 2: Okay.

Speaker 1: Um I'm going to leave under uh the on October twenty three that uh you would was talking about the the attributes.

Speaker 2: Okay.

Speaker 1: And we have done another one.

Speaker 2: Yeah, I'm gonna Yeah, I'll leave this um just calling out that like, hey, this does differ from This other pattern that we used and using a similar pattern might be helpful for translations down the line. But not being clear that it's not a required change and then uh I'll call out the my library app definition

Speaker 2: Oh

Speaker 2: Okay. I've got my other two ready to go

Speaker 1: Um have uh I have a question. So uh this the first one you mentioned it uh is uh to um to create a new ticket to um Uh just because uh uh when uh an attribute is not uh uh inside the list we are going to um set uh an um an error we are going to uh um show uh an error to um in I don't know what kind uh what what kind of error it should be, but it uh this is um what this is what we are going to um

Speaker 1: to handle

Speaker 2: Yeah. I think I think it would be helpful if model form model form options um matches model. options. in this logic of if it doesn't match raising an exception. That it it got something unexpected And I wouldn't expect that change to happen in this PR. I think that would there 's potentially a reason why we don't do that there. Um and I think having someone go look for like the a similar ticket or conversation. Um so I think just agreeing with Jern here. that opening a ticket to add an error if

Speaker 2: invalid attribute is set is I would agree with that that result or approach.

Speaker 1: Okay. But uh now is not going to to raise any error because uh no one uh uh usually um use this meta attributes or no

Speaker 2: i I don't think it will Yeah. Yeah, so Jern 's trying to ha give everyone the heads up. that if you do use the wrong name for something, it's not going to err and it's gonna pass silently. Um Yeah, and to me, I I I think if we have other places where that doesn't pass silently and it does error, we should try to match that here. So I think he um they identified uh an improvement that can be made to Django.

Speaker 1: Okay, so basically it's uh it it's something that uh uh something new that you notice.

Speaker 2: No, I did not. No. I was just looking at their conversations and

Speaker 1: No, no, no. Sorry. I mean that uh Jer Jerber uh is sorry if I mispronounce it. the the owner of the this uh this VR um also notice uh uh a new uh new things

Speaker 2: Yeah, I I think so. I I think they were looking at how the code operated and then was writing documentation to explain that. And this is a particular area that they probably didn't expect, which is why they wrote the admonition. But I yeah, I think that's a good idea.

Speaker 1: You just posted uh your your comments.

Speaker 2: I posted one on accident. I still have the other two that I'm ready to do our countdown for.

Speaker 1: I think I'm ready. Okay. Uh check the grammar.

Speaker 2: Okay. That's fair.

Speaker 1: Okay. I'm not sure it's uh can I uh can I just read it because I I'm not sure it's mm it's um It is it ca it can be easily understood. Can I can I read it?

Speaker 2: Yeah

Speaker 1: Okay. Agreed with uh sorry for the misspelling. JernGerber on opening a new issue to handle when an invalid attributes is set. Not sure of this um

Speaker 2: Yeah, I I I think that's um I think that's good.

Speaker 1: Oh.

Speaker 2: No notes.

Speaker 1: Okay, I'm ready.

Speaker 2: Okay. You 're gonna count us down? Folks, if you are listening, you might want to turn the volume down a notch.

Speaker 1: We have to to m to let them know and just gonna drop.

Speaker 2: None of the cats are in here. Dang. Alright. You wanna go three, two, one?

Speaker 1: Three?

Speaker 2: Two.

Speaker 1: What?

Speaker 2: No! All right. Oh, well, yeah. I suppose that'll be fine. I thought I'd misformatted something. But all right. I think that's uh we're gonna conclude there. Uh thank you everyone joining. Um Looks like a number of people were here the whole time, which is pretty cool. Um yeah, if anyone else would like to join the Zoom and and chat with us more. uh interactively, although we we didn't do a great job of it this time, but we do read things here in the Zoom chat a lot easier and more quickly. Uh there's a Google Form on our

Speaker 2: repository homepage where you can submit and then we'll review to make sure you're a real person and we'll invite you to the zoom. Yeah, otherwise folks are always welcome to watch on YouTube. We will put a recording up of this sometime