Peer Review and Career Development - ML 173
In today's episode, Michael and Ben discuss peer review, specifically Michael's experiences. Michael explains his unconventional path, starting with advanced math as a child, then struggling with a math-heavy computer science program in college. He pivoted to environmental studies, focusing on side projects and extracurriculars. These projects led to his first job, and later to a role at a boxing streaming service (2B) with a rigorous peer review process. Ben asks about the importance of the peer review process, and Michael highlights its value in catching errors and ensuring code quality, especially when working under pressure.
Show Notes
In today's episode, Michael and Ben discuss peer review, specifically Michael's experiences. Michael explains his unconventional path, starting with advanced math as a child, then struggling with a math-heavy computer science program in college. He pivoted to environmental studies, focusing on side projects and extracurriculars. These projects led to his first job, and later to a role at a boxing streaming service (2B) with a rigorous peer review process. Ben asks about the importance of the peer review process, and Michael highlights its value in catching errors and ensuring code quality, especially when working under pressure.
Moreover, Ben discusses the learning experience at different career stages, noting that junior developers learn from senior developers' code and feedback. Ben discusses the differences in peer review for different types of code changes. They discuss the importance of thorough review for critical code changes and many more!
Moreover, Ben discusses the learning experience at different career stages, noting that junior developers learn from senior developers' code and feedback. Ben discusses the differences in peer review for different types of code changes. They discuss the importance of thorough review for critical code changes and many more!
Socials
Transcript
Welcome back to another episode of Adventures in Machine Learning. I'm one of your hosts, Michael Burke, and I do data engineering and machine learning at Databricks. I'm joined by my incredibly intelligent cohost. Ben Wilson. I prepare releases for MLflow at Databricks.
Today, we are gonna be talking about my journey, I guess. Talking about peer review, specifically how I started working on open source stuff. Hopefully, this will give a bit of a lens into, maybe some younger people in the field and, how a traditional data science background can morph into a bunch of different things. So, Ben, I'll kick it over to you to start off with the questions. Yeah.
We're gonna flip the script this week. A lot of times it's like you asking me questions, but I wanna flip it, because I wanna hear your perspective because you've taken a journey into doing something very similar to to what I do. We've gotten here in different ways. And I really wanted to hear your thoughts because I was just I was thinking about this the other day. And I actually looked up, not to creep on you or anything.
But I was like, I wonder what like, that first PR that Michael filed against MO Flow, like, the first commit I remember. When he filed that, how many how many suggestions did I put in there? Was I nice when I was making the suggestions? And and then I looked at a PR that you filed, like, 2 days ago or 3 days ago or something, and just looking at the difference between the 2. And I was like, woah.
Yeah. This is way different. Like like, fundamentally different, in the approach and the considerations that were being made and and the thought. You could tell, you know, you do this enough and you look at enough PRs. You can almost read somebody's mind about how they were thinking through the problem.
And, yeah, it it was just interesting to see. So I was like, man, wonder what his perspective is on this. And before we started recording, I was asking you about what your journey was at your first company straight out of college. And I was very interested in how you felt that process was of how that company did things about, like, peer review on work that you're doing. So tell us a bit about that.
Sure. Let me it might be helpful to actually just start earlier. So I when I was a kid, I was very nerdy and precocious. I was into, like, advanced math. I went to this thing called the Mathnasium with my dad, to to do, like, like I think I was, like, 5 grades ahead in 2nd grade.
I was doing, like, 7th and 8th grade math, and then I moved to Philadelphia. My school was very bad. Like, I hate that place, and they basically wouldn't do any advanced type of curriculum. So I basically coasted until 7th through 8th grade, and then I started getting c's because I just hadn't flexed my trying to learn, trying to do well muscle in a very long time. And, eventually, got back to being, like, an a ish student, went to university and tried to study computer science.
And that program was very math heavy and combinatorics heavy, and I failed, like, 3 times. And then I took a sort of completely pivoted, minored in data science, and then studied environmental studies. The reason I did environmental studies is it was very easy. No joke. And I also generally like the field.
But throughout my entire academic career, I've focused on doing side projects. So for my 10th grade or 11th grade English class capstone project, I taught myself objective c and built an app. We're supposed to write a paper. And then throughout college, I tried to leverage, the amazing research opportunities and the amazing, extracurricular opportunities instead of focusing on getting a's. And some for some people, that works.
For some people, it doesn't. For me, that really works because I was really interested and passionate about things that were not covered in the classroom. So I worked with some really, really smart people, did some really, really cool projects. And then out of school, I got, via those projects, professor recommended me for a role. I got that job, and then I got another job.
And the one that Ben is referring to is, working at Tubi, which is Fox's streaming service, videos, TV shows, movies. And, I worked with a really talented data science team there and learned so much. So now that we're caught up on the peer review side, we had a pretty pretty rigorous process. I'm not gonna lie. Everything was PR'd.
There was always at least some level of seniority looking at a PR. We'd look at structure. We would look at optimizations. We'd look at statistical rigor. That was the backbone of that team's skill set.
It was a lot of inferential statistics, ensuring we're making data driven decisions that were statistically sound. Mhmm. And, yeah, I was consistently floored by the talent on that team. And after about 6 months, I was able to start giving peer review myself, mostly in SQL, and, like, how to write pipelines, but Mhmm. That's sort of the high level.
Does that start to answer the question? So the process of, like, your first poll request that you filed against your company's code base, what was that experience like? Was it gentle? Was it humbling? Was it scary?
The first one was so we did a lot of DBT and, like, SQL based DBT in Redshift SQL, for better or for worse. No comment. And the first one was probably changing docs, but the first actual PR of, like, doing functionality, I got lit up. Like, for the first, like, 3 to 6 months, I got lit up. I actually did an internship a couple summers before.
I'd came in not knowing SQL. I'll never forget. I called it numpie numpee instead of NumPy. Nice. And, like, the, like, ex Facebook, ex Twitter, super senior, like, team manager was like, who the hell did we just take on as an intern?
But by the end, I was I was writing SQL as, like, on par with anyone on the team, and worked very hard to learn a lot of things. And, yeah, I don't know. It was it was a cool time. Nice. So you get lit up, and the process of getting lit up, was that constructive feedback that you were getting, or was it almost aggressive?
It was just factual. Like, there was no intent behind it. I mean, obviously, if you needed support or needed questions being asked and as a as a kid, it's very important to, like, give that a motivational push sometimes of saying, hey. You're doing great. Or if they need to be knocked down a few pegs, maybe do that.
But the fact is the code needed to be changed, and that's what was communicated. And then the manager would come in and try to influence behavior and emotion. Good. Yeah. I mean, I would I would classify that as constructive if it's just Sure.
Factually based responses. Like, this is you know, you can talk about the tone of how somebody conveys that. But if it's just stating the fact that, hey, this is incorrect, usually, that's, for me, that's good enough for me. Like, hey. Thanks for catching this.
So it we didn't accidentally release this. That's great. So how important do you think that process is after having now been exposed to companies in your current role? And I know that this is the case, that you've seen it, where there's no peer review happening. It's just one person builds a solution, and it's now running in production.
Oh, yeah. Maybe well, maybe not well. What what is your what are your thoughts when you see something like that? It's terrifying. It's, the amount of crap that I've written at, like I'm it's a late night.
I'm tired. I'm stressed. Whatever it might be, I'm burnt out. I've been working, like, 6 day weeks for a while now, and I've written some really, really bad code. And I don't realize until looking at it with fresh eyes a week or two later, but you can't have that cycle.
You need it to be shipped immediately. And so that, I think, is the single greatest benefit. You ensure that your code at least has a sanity check from an external fresh set of eyes. It's it's really scary to not peer review. And what are your thoughts on not just protecting against bad stuff going in, but the other benefit of doing peer review in an organization, whether they're you're building, you know, pipelines in SQL or using Spark or doing, you know, you know, model building with some data science library or pure software engineering now that you're getting into that?
What do you think some of the ancillary benefits are to that? Learning? For whom? Both. So, actually, Ben, I'm cure I'm gonna flip this on you.
Obviously, both the reviewer and the writer get benefits. The reviewer, I think, learns a lot more. The writer typically gets constructive criticism about their design, but, one of the most valuable things I did early in my career was just look at other people's PRs, see the comments, see what was expected, and then started giving my own. And it would help me think critically about, is this thing a good design? What is a good design?
How are things designed? What are the principles of design? So to me, that's, like, the most valuable thing. Like, seeing there's a guy, Yuki, on the MLflow team. Seeing his code is just, like, mind blowing to me.
Like, every single time I look at it, I'm like, woah. That is how software should look, and it's it's super valuable. So, yeah, flipping it to you, Ben, who do you think learns more? Depends on where you are in your career, from a holistic perspective. So when I was a much more junior person, I was getting that.
I was like, yeah, just looking at people's code and how they think through problems and the mechanics of building the solution. And you start recognizing, like, oh, I I remember reading that book about design patterns, and that's what this is, and but applied in this unique way. And this is kinda cool to see it built out this way. And you're just sort of absorbing other people's ways of thinking through solving problems. And it's really cool, and it's I think it's super beneficial.
But if you're a junior person reviewing a senior person's work, you might discover issues in their implementation from, like, a design perspective or usability perspective that they didn't think of. So that's beneficial for them, but you're probably not in a position to provide critiques like, I don't think this is the right design pattern to use here because of extensibility in the future, or, hey. This interface is not gonna work well with this other API that a user will then use after using this API. So how would we shift this to this other approach? So in order to get that sort of feedback, you need somebody more senior than the person writing it or somebody who's just done it more or is more familiar with that part of the code base.
So but if you are that that very senior person reviewing other people's code, you're usually not making comments about style choices or you know, you know me. I'm I do annoying knits on PRs about, like, hey. I found a typo in your comment. Like, is that really meaningful? No.
But it just bugs me. So I I always, like, try to make the comment and the suggestion, just fix it for people, because I know people are typing fast, and a limter is not gonna pick that up. But, usually, if I see how somebody implemented something and I it's not how I would do it, I'm like, well, I wouldn't I wouldn't use a data class here. I would use this other structure. Irrelevant.
I don't care. Does it work? Is it still giving us the benefit by using this other collection type? Perfect. It's fine by me.
Stamped. Looks good. Because I know it's not it's not constructive to them to be like, hey. You don't have the same preferences that I do, so this is wrong. It's not correct.
Right? It's just we have differing opinions. I understand what you did, and it's viable, so I'm fine with it. Question, actually. How do you align on standards for peer review?
I've really struggled with that. I've seen it attempted before where, and we even have them at Databricks, by the way, where there's like a a company style guide or a a set of standards for review. I've never seen them adhered to because it's so much mental gymnastics you have to go through as a reviewer to check that guide and be like, am I hitting these? Because that when you create one of those guides or anytime I've seen somebody create 1, and, by the way, I've created 2 of them in my career. I will never create another one.
It starts off with 10 bullet points. But, like, we're gonna adhere to this coding standard. We're going to use this linter on this version, and we're going to, you know, use this design approach and everything that we do. It starts off with those 10 bullet points of whatever you come up with. And then a year later, it's there's, like, 300 bullet points that have been, like, added to by other people because all of these these, like, conditions come up that we hadn't thought of when we first created the doc.
And it it becomes a sort of, like, reactive mechanism to be like, well, we're gonna prevent this people making this mistake by just adding more crap to this document. And, eventually, the document gets so big that nobody reads it and nobody cares, and then it just dies. Right. So the the better way to do stuff like that for adhering to standards is just get a really good lender that can refactor code and point out where things don't work well, and then have make sure that there's people who care about maintainability standards and readability standards in the code base that are coming in and making positive comments that are reflected in that that vein. Like, hey.
This function is too complicated. Can we break it up into, you know, a different maybe tackle it from a different angle to get the same functionality, break the function up into 4 different functions, or, like, hey. This one function is doing 17 things. I can't read it and understand what the state is in the middle here where there could be an issue. Let's separate these concerns into, like, a more maintainable and testable state.
So you just need somebody who's done it enough to come in and make those suggestions for simplification. Okay. But getting a group of people to all align together on some sort of review standards, I've never seen it be successful. The way to solve it is get a group of people together who all look at implementations from slightly different angles and tag them all if you have capacity to do that. If you don't, pick the person who you feel will be the most critical of your code Mhmm.
Or the people. So if you're in a team of 10 people, you're not gonna tackle 10. Nobody's got time for that. The whole team wouldn't get anything done. So who has the most context here about the type of code that's being written?
Like, oh, I'm I'm generating a an interface to a remote procedural call, and I need to make sure that this rest interface is done properly and that I'm applying the correct prototypes here. Get somebody who's built protos before to review that. Like, who contributed the most to this part of the code base? Make sure that they're tagged. And that's making sure that you're gonna get somebody who's gonna be very critical about what you built.
And you're also probably gonna learn something from them, so it's a win win, and you're gonna ship something that's probably gonna work. Don't just grab somebody who has no context and is looking at it from sort of a a technician's perspective of, like, yes, I think this executes without throwing an exception. It's not that helpful. Because all they're gonna do is be, like, yep. It it looks good to me, but I don't have any context on this.
So responsible engineer would would be tagged on something, and it happens to me every once in a while. Somebody will tag me on a on a PR, and I'll look at it, like, yeah. It it looks like it'll run, but I don't know what the downstream blast radius is of this. Let's and I'll just start adding additional people that I know will know that, or I'll explicitly ask them, like, hey. Can you can I get your eyes on this to make sure that we're not doing something crazy here?
Okay. That I have, like, 70 questions on that, but I think it would derail a bit. But, yes, that makes a lot of sense. So shifting from your time at Tubi and being in a situation where your SQL is getting reviewed through through a peer review process, and people are are helping you along the way to ship good code that you're also learning from at the same time. So that that whole process is a continuous learning thing.
Right? Yep. Because I mean, I'm sure there were times where you submitted a PR that was if we're talking SQL, that could be 1,000 of lines of instructions. But let's say in Python, right, You're shipping a PR that has 500 lines of code change. I'm sure there are times at that organization where the only comment you got was from a team lead saying, looks good to me.
Let's merge. Yeah. But there are other probably times in the last part of your time that you were still working there where you'd ship the same number of lines of code, and you'd still get feedback. Right? Some people, like, yeah.
I don't know if this is the right way to do this or Of course. Yeah. Think of this. So it's always that continuous learning. You move from that, and you go to the field at Databricks as your next job.
How much peer review did you see? Like, negative 0. So, the field is, in theory, spread really thin. I think it actually is spread quite thin. There's a lot of customers that need a lot of help, and, Databricks has an interesting market niche where there's, like, the upper echelon of digital native people that are really, really skilled, really know their tech stack, really savvy in math, data science, you name it.
But there's a lot of customers that are not that way just as a product of what we do. We may help make data analytics and advanced ML and things like that easier, And every customer wants that. So I specifically work in the retail vertical, and we have some really advanced customers, but we also, don't have some advanced customers. And so, yeah, my team is constantly bouncing around, trying to provide as much value in the short amount of time that we have. And the skill set is actually kind of weird and hard to hire for, I've I've learned.
After 6 months to a year, you're great. You're golden, at Databricks. But, coming with the Databricks skill set and the ability to learn that quickly is actually sort of challenging to find. And so in summary, there's very few projects where we have multiple people on them and even fewer where peer review is deemed a very high importance activity, which I disagree with, but it is what it is. So did I when I was in the field as well.
And at the time when I started, there were 8 of us that were doing all of it globally, And there just was no mechanism for, like, hey. Could you stop that high priority engagement you're doing with this customer where you're on-site right now to look at my implementation that you have no context of, like, what I'm trying to do here? It just never happened because it it's untenable. It's almost sad to see that that hasn't changed in in years, but it's sort of the the way that I think a lot of consultancies work. Now there are some consultant groups out there that send teams of people out, and they have, you know, a well proven, you know, software development mindset that there's like, hey.
There's a team lead going out there and then 4 engineers that everybody's reviewing each other's implementations. They're working on a repository together, and they're doing, you know, proper development practices. But the vast majority of consultancies out there don't do that. It's 1 or 2 people that go out there to just pound out some code as quickly as they can. Does it run?
Yes. Good. Contract complete onto the next one. And sometimes because companies are left hanging with a code base that they can't read, they don't understand, that's flaky or just full of bugs, and they're sometimes unable to fix it because the implementation was so convoluted, just incredible spaghetti code. You're like, oh, I tried to refactor this, but the refactor turned into a rewrite, and I don't wanna do that.
But I can't break this. I can't extract these three methods from this this class because now I have to create 4 new classes and share state across them. And yeah. See, that sort of approach where nobody's looking at somebody's implementation because they're just trying to bang it out as quickly as possible, I think that's pretty rife in the consulting industry. For sure.
So tell me a little bit about how you've been trying to change things immediately in your team right now. So I wanna be clear that it's project to project. I recently did a project with a large media company and worked with a truly phenomenal, data scientist. Like, I gave some feedback. He's probably the best builder at Databricks that I've worked with in the past 2 years.
Just like straight up builder. Like, you give a problem, hear the components, architect a solution. And we had really intense peer review, like like, kind of comically intense. And then we after we aligned on standards, subconsciously, we dialed it down a bit. But some projects are really, really rigorous and robust.
What I've been trying to do is basically work with my immediate team, specifically new hires, to create sandbox environments where they can make simple mistakes and start developing intuition. Because following, like you said, a bullet list of 300 things that makes good code or bad code or whatever, that isn't really sustainable. It doesn't transfer to new areas. It doesn't, you can't really leverage that as a building block for knowledge. Instead, learning something, quote, unquote the hard way, hopefully, in a safe environment, that is a building block for knowledge.
Mhmm. And so I've been working on some pro bono work where we just donate professional services to nonprofits. And there's one project that we're currently doing. We're building basically a large data ingestion pipeline with a bunch of Gen AI stuff to augment a really large medical database. So it'll do feature extraction with LLMs and things like that and then put a rag chatbot on top of it.
And so with that, new hires get 40 hour projects, and they get peer review from more, season Databricks RSAs. And watching their growth has been kind of mind blowing. I've I've botched so many times, like, giving instructions, providing too much guidance, too little guidance, etcetera. And I think we're actually starting to hit our stride a bit, and it's just amazing to see. So creating these sandbox environments where you understand what is good code and start developing intuition about how you should approach these problems, it's really, really effective.
So these solutions are an application that's being built, and it's more of a, like, hey. A main process runs this thing. It's scheduled or Correct. It's triggered some way. When you're when you've gone through the process of doing peer review in something like that compared to your now other side projects that you're working on, which is open source con you know, contributions, how would you classify the difference is in the intensity and the amount of things you have to think about when you're not building application logic, but instead you're building framework logic?
Yeah. So at a high level, the way I interpret those two terms, application is an applied use of frameworks typically, and then frameworks is a it's a tool, essentially. So instead of building a house, I'm building a sledgehammer. And so for the framework code, it is night and day. I I do have a style guide on the read readme.md, and it's, like, 10 bullet points about the principles of how we're gonna approach designing within this repo.
And every single I use it as a template for every project. I start every single time it gets ignored. I should probably just delete it. And at a high level, we look to create reasonable unit test coverage. But if the workflow works, we typically don't do that much more.
We're not working, for a lot of my recent project, we're not working with very large ins like, super intense production pipelines, and it's more of a POC that we then hand over to the customer, which is often the business model. And so I've shipped projects with no tests. I peer review the crap out of it, but, if it's just spark code, it's kind of overkill to mock up all the tables and then, see if stuff is working end to end. So on the flip side with OSS, you really like, I spent, I don't know, like, 2 hours deciding how to pass parameters for from crew AI, or sorry, from the user through a Unity catalog create function to a crew AI tool, and I will spend a few more hours on it today finishing up that logic. In application code, that that concept doesn't exist.
You're just I don't know. Make a constants file, put a global variable, just figure it out. It's hard coded. Yeah. Yeah.
A computer's gonna execute it. It's not a human. Yeah. So what's the experience like for you that in specifically in the peer review process between those 2? Like, even if you're working with the best, you know, data sign like, applied data science person in the field, and you're doing intense peer review of an application that you're building versus you're contributing to an open source project that is public facing APIs, basically.
Like, a user is gonna just use this API to build. We have no idea what. What's the difference there? Like, do you have do you find that there's different questions being asked or different challenges that are happening? And how does that how has that adapted your thinking into your other work that you do?
Alright. This is, like, a big question. For application code, you need it to work once and you need it to be extensible and maintainable. Or not work once, but work for the use case that is provided. Fast one, some good shit.
It could work tomorrow. No care. Yeah. I'll be off the projects and not my problem. No.
It it needs to work reliably in the context that it was designed for. For, framework code, it needs to be dynamic and intuitive and, ideally, super usable. So that's a whole different problem, like, completely. You need to really put yourself in the the customer or the user's shoes to design something that is stable and, if I come from this background, would I use it this way? How are other libraries designed that are similar?
So it's it's like apples and oranges. They're completely different. Yeah. And have you noticed that I mean, because you've done quite a few contributions now to packages. You got a couple PRs in flight.
You're starting another one next week Yeah. For Unity catalog integration. And you'll probably fin like, start another one right after that. So has that process of the entire dev loop for these open source things? And what I mean by entire dev loop is, Ben sends me message saying, hey.
We need an integration with this toolkit, like, with this framework. And that's pretty much all the context I give. Maybe a link to a PR that we just merged to be like, hey, kinda this is what we're thinking, but that's all the guidance you get. That dev loop of from super vague guidance to go research, figure out what the integration is, build a demo, like a proof of concept, then go work on the implementation, write a ton of tests, file a PR, get a bunch of feedback, fix, and iteratively do that, has that whole process influenced your other work as well? Without a doubt.
I'm trying to think about the the biggest thing that I've fumbled in this, like, mentoring concept and, like, teaching concept is assuming that what I now know is correct and is, like, known by other people. I like, when I first started doing this, I I just lacked the empathy to remember where I was a few years ago, and I, like, botched a few things. I was like, oh, we need this feature. Go build it. Let me know if you need help.
And then, like, a week later, they come back and they give this, like, abomination of a code base that just doesn't work and doesn't do the thing that we need. And I'm like, where where did I go wrong? So I think that is one way it's really negatively impacted me. And since realizing that, I have corrected, and now I'm a I think better. But it's really it's a tough moving target, especially if you work with someone for a very small amount of time.
But I typically play the TL role in a lot of my projects now where I do that initial design, that mock up, and provide a design doc to someone to actually implement it. And if I'm doing it end to end, I can do it so much faster. Like, this crew AI integration, I was actually thinking about what are the tips that I wish I knew when I was doing the llama index flavor. And the llama index flavor took me, like, legitimately 6 months of, like, actually working on it a fair amount. And, I learned so much along the way, but there's just, like, basic things that are really valuable.
Like, don't close Visual Studio until the PR is merged. Like, just leave it open. There's gonna be a comment. You're gonna need to refactor something. You're gonna need to rerun an example.
Just leave it open. Minimize the window. And that has saved me, I don't know, like like, 20 hours in the past month, just like that lesson alone. So it it's weird things like that that I'm a lot more efficient with my tooling and a lot more efficient with my process. And then finally, I think I'm a lot better at helping people at different parts of this journey.
Yep. But I at first, I sucked that at. I think I think everybody does. I did as well the first time I had, you know, direct reports in my career. I think everybody goes like, makes assumptions.
Yeah. Sometimes they're from a bad place where you're just not empathetic at all, and you just don't care. And sometimes they're from a good place where you're like, oh, I don't wanna insult these people and make it seem like, yeah, I think they're idiots, so I'm gonna just make these higher level statements that, hopefully, they'll be able to figure out. But if you don't know it, if they have that level of context, sometimes you're like, alright. Maybe I just put this all down in a document and not tell them to to do this, but tell them to reference this So that they can go and read it, and then they can get up to speed, and we're all on the same page.
That's our whole, like, design process that we do. That's why we do it. It's so that we're not putting people on the spot and giving people time to think and absorb and process stuff. And yeah. And when you're mentoring or interacting with people at different levels of their career, it becomes interesting at a certain point.
If you have a team of, say, 5 or 6 people, and all of them have, you know, say let's say, 10 years experience, Not all 10 of those people are gonna have the same experiences, even if they were have been working together for 10 years. People are going to naturally focus on things that they their own brain finds more interesting and important, and they'll get really good at what they really like to get good at. So and they're not all gonna do the same thing. Right? So it's not like, oh, we're gonna have 10 people that are just amazing technicians.
They they're like, when I look at the code, it looks like just poetry on the page. You're gonna have 2 or 3 people that are like that, and you're like, wow. This is this is impressive. Very clean code. Very well designed actual implementation.
And then you'll probably have 2 or 3 people who the code might not look perfect. There could be some comments that are just purely technical in nature. Like, oh, well, you didn't use this operator in the language, or you we could have written this, like, comprehension a little bit more efficiently, or that's not how we should be doing type checking here. There's so many examples that you can provide for stuff like that, and that's, like, knowing the language that you're developing in. But those 2 or 3 people are gonna spend a bunch of time before writing their first line of code, figuring out what the interface should be.
And they'll think about like, they're really good at thinking about it from a user perspective about thinking like, product focused development, and they they excel at that. And then you might have some other people that are generalists. They're, like, really good at every aspect of the process, but they're not the best at 1 or 2 things. And when you're providing advice for those people, typically on, you know, private one on one conversations where you're talking about, hey. Here's where I think you should go next.
About when doing mentoring with with people that are senior enough, it's not so much a a point of advice with them. It's not like you're telling them, hey. You need to improve here. Because that can come off as, like, pretty insulting, because everybody can improve in anything that they're doing. You don't need to state the obvious to to seasoned professionals.
It's more like, I think it would be good for you to do this part of this process. You know that they're not really that good at this thing. Like, that maybe the people that are extreme technicians, you give them the product design task to do. And they might seem a little panicked at first. They might be like, can you can you review this privately?
I'm like, yeah. Sure. I'll check your work before we send it out, beef before anybody else sees it. But helping them grow in that way and leveling them up for the the parts that they don't naturally focus on can help improve. And one of the ways that you can do that with somebody, in a in my experience, in a in a very easy way is the process that you've been doing.
Just like, hey. Contribute to open source. Like, work with the team that has to think about these things, and you're gonna gain that knowledge. Whether you want to or not, you're gonna be exposed to it. You're gonna get those comments, like, how is the user gonna pass this?
And they're like, oh, yeah. How will that work? Now I gotta go think through this. Yeah. The Socratic method is really good.
And it also made me think of a conversation we had with my my friend Danny of, like, a few episodes ago about finding that entry point into someone's learning or someone's ability to learn. And, yeah, the facts and feedback can be delivered very differently, and I'm super freaking direct and super, super blunt. And so I often have to, paraphrase or, like, do the sandwich method of compliment, feedback, compliment, or whatever it might be. And I think that delivery is incredibly important, and it's just hard you need to, like, have EQ, have practice, understand what is professionally acceptable versus, like, friend acceptable. So, yeah, it's it's just a really freaking tough problem.
Yeah. I agree. Full like, completely. And sometimes you can get feedback, like, the the open source packages that you've been contributing to. You have the benefit of, you know, the people Exactly.
Are look like It's very different, yeah, if it's a stranger. And I would I would quantify our team as both professional and pretty nice Oh, yeah. About feedback. I've contributed to other packages where I get a, like, a totally different vibe, where it's like that sandwich method, where it's like the person you just see this big paragraph of text, and the first couple of sentences are talking about, like, them thanking me for doing the contribution and saying, like, oh, this is really great. But and then they they say what the problem is.
And then at the end, it's like, thanks so much again for doing this. This is so great that you're like, dude, you could've just told me the middle sentence. Just tell just tell me what's wrong. I'll fix it. It's no big deal.
Do this every day. Just tell me how I I screwed up the integration with your your your framework. Like, no biggie. And I've also experienced the other side, which is, I would say it's the sand it's it's a sandwich minus the buns, so just the meat. But I think that's how we do stuff.
Usually, just just stating the fact. But more the instead of meat, it's rotten meat. So it's just hostile. And somebody's comment is, like, you can read between the lines, and they're they're omitting their own sandwich, which is, yeah. Thanks, jackass.
You're an idiot. Here's the problem. Try not to be so dumb in the future. I mean, that's kind of the tone of what it is, and I'm fine with that. Like, that's a day in the life of me is, you know, doing something, finding out I screwed it up, and fix it, and having that persistence of just, like, I'm not gonna stop until I get this right.
And I'd say that's the same with everybody on our team by far. Everybody just wants to to get stuff done correctly, and nobody really cares how that message is delivered. But that hostile thing, I hate seeing that in open source packages. Like, some maintainers like that with a contributor, because you just turn that person off from your project and probably all other projects. Like, you just destroy that person, by being, like, such an a hole.
How do you feel about stuff like that? Or have you ever experienced that in a peer review process where somebody is just, like, not a good human being? I don't care. I personally don't give a shit. Like, I'm gonna do the work that I came here to do.
You can be an asshole about it. You can be nice about it, but you're gonna give me the feedback so that I can get my job done. Full stop. That said, if it's like, willpower and motivation do have limits. So for longer term things, sometimes, like, encouragement and positive feedback is important.
But, I was trying to think back and nothing is registering. Like, I've definitely been, like, borderline yelled at a few times. But it's as long as the content is there and the person is generally right, that's the main thing. Like, ideally, don't, like, curse at me and, like, say, like, I'm so smart and nice. Like, that would be cool.
But as long as you're providing the correct feedback and it's an opportunity for me to learn, I'd it doesn't clock. It doesn't register. So that's that's you. Do you think everybody is like that? No one is like that.
Literally, no one. I'm not saying our whole team is like that. Like No. I like, doesn't care about the tone that something's delivered on the MLflow team? I think no.
There's a 100% of tone on that team. Yeah. There's a tone that we all adhere to, but I don't think anybody would be I'm talking about, like like like, I used to work in, like, the finance space a little bit. Like, if people throw shit, people, like yeah. So to be clear, like, I think them offload team does have a very kind and direct tone.
It's like the most efficient like, kindness is taken for granted and and as it's expected. And so with that underlying, like, cushion, you can then be very, very direct Yeah. And maybe throw a smiley face emoji in there, and it's well received. Yeah. But there are definitely like, there are many ways that I could leave feedback and people on the team would not like that.
Oh, for sure. I I mean, because they know you. I mean, you'd hear from me first. I'm like, dude, what are you doing? Go delete that comment.
Yeah. Just causing chaos. Yeah. Yeah. That that would not go over well.
It's because of the culture that's been built, and that's not just our team. It's in the entire organization of, at least on the ML side of the house. People just interact with one another that way. We all know we're on one team. We're all working towards one mission, and we all like each other for the most part.
We're just trying to just like, yeah. I know that guy or I know that that lady. Yeah. I wanna wanna help them out. You know, it's it's just it's all it's not unprofessional friendliness type thing.
It's more it's professional friendliness and just being really respectful. No. I I can actually give you the sequence. I've done this a 1000000 times. The first time you offer to do something like, yes.
Great. So helpful. Here are the 7 things that you need. Go do it. And then when I come back with something that's just fundamentally rotten and broken, and it will take a lot of that person's time to fix it or help me understand, then they start dragging their feet a little bit, and that's when the personalities come out.
Depending upon the person, sometimes they're like, yeah. This is what you should be doing, or they'll, like, rewrite with a simple example. And then other times, they just won't respond. And Yeah. So it's like that's starting off of of openness and welcomeness with the undertone of of kindness, but also the undertone of I have a 1000000 things to do.
You were going to help me by doing this, but as soon as you stop helping me by shipping crap and asking me to fix it, now there might be an issue. And no one's ever directly, like, mean about it, but, yeah, they they got more important things to do than, like, teach me how to, like, write a 4 loop or whatever it is. I mean, sometimes there is time to do that, and, like, I try to help out for stuff like that because I probably have the most amount of wiggle room Yeah. In sprints. But I can't do that for every scenario.
Like, definitely not not with the volume. So it's being selective and knowing if you see 15 things wrong with somebody's implementation, can I give just one response that's gonna unlock their brain to be like, I'm gonna go find those 15 things that that he didn't even mention and and go fix them? By the way, you do that all the time. Sometimes it's only it's not like you ship a lot of bad code or anything. It's just sometimes when you're a little off the mark and we're not, like, right before merging something, So it doesn't need that intense scrutiny of, like, okay.
I need to go line by line and mention everything that needs to be fixed. It's more like, I'm gonna just give him a hint here, and I know that the next commit is gonna have all of those things fixed. And I don't even need to mention the other things, and you do it every single time. And it's just like, yeah. I knew he was gonna do that.
But some other people are not like that. Somebody that we're talking about before we're recording is definitely not like that. And you have to spend that extra time of going through and and just every single instance writing it out to the point where you're like, okay. There's there's a 140 comments on this PR. This is rough.
Yeah. And, yeah, if if it's something that has to ship, sometimes you get that that sort of delayed response, or somebody just stops interacting with what you're trying to do. They either means they're super busy, and they they have something that they need to ship that's, like, they're being measured against, or there's a deadline, like, tomorrow that they need to have this done by, so that's what they're working on. They've shut off communication. Their Slack is paused.
They're not checking email. They're just in their IDE, banging code out. Or they don't wanna help you. Yeah. Or they or they don't have a a delivery, and they just look at it, and they're like, yeah.
I'm gonna go check with my TL and see. Should I just take this over? Should we tell this person to close this PR? I'll get it done in 2 hours, and the feature will ship. So, yeah, you have that type of stuff that's sort of behind the scenes.
You don't see it going on. But and there's other people that are just like, yeah. I don't I don't wanna do this right now. Not my job. So but we we try not to have people like that.
There's not really people on our team that are like, oh, it's not my job. I don't care. It's more like there's so much stuff going on. They don't have time. Exactly.
They're gonna make time. They're gonna work later that day just to give you that feedback. But it it's, they're making a sacrifice in doing that because they believe in you. Yeah. I've I was actually reviewing something yesterday, and, there was a bunch of comments from multiple people and, basically, it was not directionally correct at a high level.
And I didn't respond to them because I didn't really know where to begin. And now I'm like, I I instead decided to do, like, a over a call meeting, to just quickly realign. But Mhmm. Yeah. I've I don't think that there's many people at Databricks period that are just like, fuck this dude.
He's an idiot. I'm not gonna help him. It's more like helping him is gonna be a lot of work. I hope that he gives up before taking more of my time. Yeah.
I mean, depends on where you're committing code to. Fair. That is that is valid, actually. I've had a couple PRs. Yeah.
So you've been on the our open source side offerings. Right. And that's a very broad, you know, code coverage base. Right? We're talking about even if you're just talking about MLflow, a lot of the stuff that you end up working on is stuff with, like, the models and Right.
The user facing type stuff. Like, it's back end code, and it it's complex. But you're not making, like, API changes Right. To public APIs or doing, like, the back end back end. Like, oh, I'm changing how the rest interface interacts with the actual database.
Those changes, if you were making changes there, you would get a different set of people making the comments, and they would be, like, very pointed and, like, overly critical of what you're doing. And that's because if that breaks or is that if that's not robust, the entire tool just doesn't work. If the model flavor is a little buggy or is doesn't quite feel right, it's still super important, but it's not as critical as, like, fundamentally breaking the tool. So you get something different. And a corollary to that would be on the internal code based side, when we have to make changes to that stuff, we get, like, intense scrutiny of anything that's being done with questions about, you know, the our mono repo is gigantic.
I don't even know how many lines of code. It it's shocking. And if you're adding something that isn't in adherence to existing tooling, if you're trying to reinvent the wheel, nobody cares that you spent the time doing that. The people that are reviewing it are in a different department sometimes. They don't care how you spend your time.
They just wanna make it so that you're not building something that already exists that's a departure from a standard that's been built. So if you're just ignorant about that, they'll respond very pointedly, and provide a bunch of links. And when you see links in your PR, you know you've really messed up. They're like, okay. I I didn't do my homework.
I didn't see if this was something that already existed. And but the people are doing that for two reasons. 1 is to make you aware of it and to get your code to change, but also so that, you know, a year from now or 2 years from now, when that underlying, you know, shared library changes, your code is now not a departure from that behavior. Right. Because you only wanna chain like, make a blanket change that affects 6,000 services.
You wanna make it one place at one time and then make sure that everything works. But if everybody's building copies of the same functionality 6,000 times, We're talking about, like, a security audit. You know, like, oh, jeez. The way that we handle, you know, temporary credentials needs to change. How many places do we use temporary credentials?
Yeah. Whatever it is. 57,000 locations in the code base. If each of them had their own bespoke implementation, that's 50 to 7000 PRs that they'd be filed. Nobody's doing that.
So you're saying going back to an earlier point, you're saying the best place for this standardization knowledge to live is in someone's head and not in a document? Or in in the existing code base, I guess, plus someone's head? The best documentation is in code, in my opinion. Like, if it's something that you need additional context, and that's that's another good tip for us to discuss. You see it in our PRs every once in a while, where we'll either ask somebody in a review comment to say, hey.
Can you add a note to Bene here that references why we're doing this? You probably see it from me, Haru, Yuki all the time. Mhmm. And you see it in Yuki's PRs where he'll just preemptively put that in there. And that's a that's a byproduct and a consequence of somebody suffering through a very large complex monorepo and getting comments, like, why aren't you aware that this exists?
Please use this. You know, like, how would I know that that that exists? Like, unless I gripped the entire code base for some unknown parameter that I don't I wouldn't even know to search for. So the the only two ways that you can solve that problem of understanding if what you're building seems generic enough that it should be some utility is to go and ask the team that would be the ones that would create a utility and say, does this utility exist? Super inefficient, and you're not always guaranteed to get a correct answer depending on who's gonna answer that question.
But if you have it in code and you're looking at something that you're gonna modify and you're like, oh, there's a note here about why this exists with a link to a document that this person either wrote or somebody else maintains that I can then go and read and understand, like, okay. Now I get why we're doing this, because it seems weird. But with this additional context, okay. It makes sense. And I do prefer those document links if the concept is super complicated.
There's nothing that annoys me more. I've seen it in code bases before where it's like, okay. This is a page full of comments of just, like, an entire like, a 150 lines worth of comments explaining why something is the way it is. So you want a Databricks internal document for OSS? No.
That would be like a public document that you could view that's hosted on Google Drive or something. Got it. But if it most of those comments, by the way, in MLflow relate to Databricks integrations, because in open source, if it is something that needs some sort of special note about why something works, either it's related to an integration we're doing with another package, then link to their docs on their website that explain how this thing works. Or if you need to put a note in to explain your implementation in a stand alone open source project, you did it wrong. Because the code should self document if you're the one in control of everything.
So if you see a a note based on, oh, I need to integrate with this thing that I built and maintained, that means that's a hack, and you shouldn't do that. Like, change what needs to change to make this more self explanatory. Yeah. But, yeah, definitely prefer that a lot more than the oh, you should be able to get this context for me. It's in my head, and good luck.
Next person that's gotta, you know, modify this. Yeah. No. Agreed. Reading code is a really valuable skill, and it's not something that's always super intuitive.
Like, it I think I'm a lot better at it. It's still a lot to learn, but at first, it was tough. It was really rough. Okay. I mean, it's like learning another language.
Yeah. Literally. Like, when when you start it's been so long that I don't remember what the sensation is like of looking at, for instance, like, Python code or Scala code and looking at it and be like, okay. I gotta really think about this and trace this through and see, like, what's going on here. And then, okay, I need to go and find this this definition and then really trace, like, what is this?
Now it's more like at a glance. I'm like, oh, I get what this is doing. Yeah. But Exactly. Yeah.
It takes years of building, reviewing, investigating, debugging for you to get that sort of mental model. Like, oh, I can grok what the code is doing here. Yeah. Yeah. It's cool.
I'm at the place where I can look at any script and fully understand it pretty perfectly, and now I can do basic design. So I can do, like, a couple levels of abstraction, but I can't do a full module or, like, a full package. Like, the the o open source, Unity catalog AI connector thing. Uh-huh. I just barely understand why these decisions were made, but, like, just barely.
And it it like, of course, I can read it, but I was thinking, could I have arrived on this design myself? And it's just barely a no. Which part of it? All of it. Like, the client abstraction?
Or Yeah. The the client abstraction, I think, wouldn't be that bad. For instance, like, we were talking about this literally yesterday of putting a TOMO file without any shared utilities in each integration, subdirectory. I didn't I mean, it makes perfect sense now, but I would not have known that that would make sense. So certain things like that.
For context for the listeners. Yeah. True. It's so that if we make a change in the core package that the integrations depend on because they import the core package to to do things. If we change the the core package and modify the behavior of the utility, we now have to release all integrations even if they don't have any code changes because of that tight coupling.
It's just a maintenance burden. Yeah. So instead, we copy and paste the exact same thing for each new integration, but it's more efficient. Well, it's not copy pasta. It's it's close.
It's a distinct implementation because it's a pandemic model. So each one has different fields, but the base implementation of that is identical effectively. Well, it's named the same for familiarity. Mhmm. But the object itself for each integration is completely different.
Yeah. So, yeah, we could have abstracted that and created a bunch of, like, abstract methods that we have to override or created an actual instantiation of that based off of an a b c that defines all of those, and we're just mutating it when we import it into each of these integration packages. And you'll see that design in other packages that are doing maybe not this exact thing, but similar things where somebody made the decision of, hey. It makes sense. Like and it's more on the the actual development velocity cycle.
If you have it set up that every time you're releasing any version change for your entire collection of libraries, everything gets released automatically all at once, then, yeah, it makes sense to do that. But that entails setting up an entire, you know, deployment pipeline that can do all that. But if you're doing it manually or you're triggering scripts that do stuff, nobody wants to sit there and, like, manually push out, like, 18 different integrations in an afternoon. It's just annoying. And the more repetitive task that you do over and over, the higher the probability that you're gonna mess one of those up.
Yeah. Exactly. Cool. Well, we're coming up on time. Any closing thoughts before I summarize?
No. I think we covered some some interesting things. I got some more insight into how to improve my own empathy on on certain things. This is useful to me. Sweet.
Yeah. It was not useful to me at all. Good. Alright. So in summary, we sort of went through a bit of my journey.
There's still a long way to go, and I'm very excited about it. But, some things that stood out to me in this episode. 1, if you're early in your career, read PRs deeply and read other PRs and read the PR feedback. It's a really great way to understand what matters when reviewing and what matters in design. When teaching, try to have empathy and meet the person where they are.
This is super important. Direct feedback is really valuable, but direct feedback should look different depending upon what the information is that that person needs. Some things that I learned the very hard way is get good at prototyping. If you can quickly mock up something and learn the signature that you need to create or whatever it might be, that is really valuable. If you struggle with that, it's really hard to build a good software and also learn your tools.
They're there for a reason. And then finally, different teams have different cultures, but direct kindness is super efficient. That's what the MLflow slash OSS team at Databricks leverages, and I love it. It's super efficient, and, everybody's been really nice. And So you ask them for too much help, and then they ghost you.
They're just busy, man. No. I know. Yeah. Yeah.
I I do the same thing. Yeah. Anyway, anything else before we close, Ben? No. Well, until next time, it's been Michael Burke and my cohost, Ben Wilson.
And have a good day, everyone. We'll catch you next time.
Peer Review and Career Development - ML 173
0:00
Playback Speed: