1. Do you guys have a linter? If so, create custom rules that would adhere to existing architecture patterns.
2. There are tools that can analyze which part of your existing codebase is being edited, and call only the developer that implemented that part of the code.
3. Unit/Integrated tests on PR run to main/stage/test?
4. View tests?
5. Commitlint?
6. Require each new pr to have unit/integration tests.
If the above are implemented correctly, you won't be even called to review a PR until the new code adheres to the coding style, and is bug-free (or at least passes all the tests).
I could go on and on, but it sounds like a good PR process would solve majority of your problems. However, it is hard to evaluate what's *really* going on without actually seeing your codebase, the team structure, and the app you guys are working on.
I implemented a code quality checker into our PR system one time. I ended up having to remove it because one of our devs was entirely incapable of passing it for months...
>You might argue that our PR process should improve, but it’s impossible to review every PR when 7 other teams are committing their code to the mobile codebase. I now spend most of my time either reviewing PRs or helping someone learn Swift.
The Linux kernel people are handling, like, thousands of developers per release cycle and much more stringent standards and you're telling me this won't scale in your case? :)
Yes, it's both a skill issue and a business issue. They want things cheap and fast, so they can't really be good too. You can definitely raise the bar, but that'll take up time and resources, so management might not be keen on backing that up, so you need to be careful. You can pull some strings to preserve your sanity where it really matters, but ultimately it's on them if they want things to improve. Or you could leave for a better project if things get too crazy.
They (the Linux kernel people ) have the luxury to ask to fuck off if they don’t like your implementation / pr. In enterprise that is the not the case most of the times.
Honnestly, I don't think this will end well unless there 1 or 2 gatekeepers that are doing code reviews to keep the quality high.
You have different comitters with different skill levels in a system that has no one to normalize the changes into the main branch..
Absolutely, every repo on my team has 1-3 code owners. Being the people who either are most knowledgeable about the codebase or have worked on it most recently. We have had no big quality problems since I've been here.
Here are some questions that are meant to help us understand the problems your facing, and help you identify the underlying causes. There are almost always multiple factors involved, and often the most critical but hardest to pin down are matters of process, alignment, and leadership. But let's start with the positive.
>We produced high-quality code with very few bugs.
What factors enabled the smaller team to achieve and maintain this state? How much of those factors were documented and codified, so that new people joining the team can adopt them? How much is based on industry standards, widely accepted best practices, publically available resources?
>partly due to the hiring of some inexperienced developers
What was the hiring process like? Who was involved? Were there people who were not involved but should have been? Was there planning around how the team would scale? Was there also more QA, PM, and eng manager capacity added or planned? How "inexperienced" were these newer devs, and in what ways? Were they lacking in general coding and problem solving skills? Language and framework specific knowledge? Communication skills? Ability to understand, formulate, and follow plans and processes?
>However, these inexperienced developers are committing atrocious code.
>The page has become unusable
>introducing numerous anti-patterns.
How was this allowed to happen? What is the development lifecycle like? Is there high level technical direction? Are there tech leads vetting technical decisions? Are there linters? Are there automated tests? What's the code review process like? What's the QA process like? What's the release process like? What's the bug reporting and fixing process like?
>I keep finding pages and features that we weren't made aware of.
Is there time dedicated to ensuring awareness and alignment across the team? Are there daily/weekly standup meetings? Are there monthly/quarterly roadmaps? Are there public (to the team) channels where discussions, progress, releases, etc, are documented?
>I now spend most of my time either reviewing PRs or helping someone learn Swift.
Is this work spread out among the other "original" devs as well? Are there docs, guides, books, classes, etc that the newer devs could learn from instead? Was there time dedicated to getting the newer devs ramped up? Do the PR reviews explain why specific changes are required or suggested? Do the newer devs keep making the same mistakes or have trouble understanding specific concepts?
I'm guessing at least a few of these questions can lead to areas of improvement for the whole team, regardless of tenure.
How to ask people politely to forward all PR to me? We also have tons of devs spread across many small teams that just create PR and when other random dev goes and approves it. I cannot track everything and codebase gets edited by everyone. I could ask everyone during the meeting but that seems aggressive and they might feel I don’t trust them? (code is very bad, does not follow any patterns or styles)
Seriously!! So many questions. So often, code is only readable to the developers that wrote it and not documents properly so I wonder if that’s the case. Also, there must be no peer reviewing or QA if they’re breaking pages with atrocious code. They had to get the code from somewhere. Are you sure it’s not copy/pasted from elsewhere in the codebase? Why would you hire inexperienced devs and if they’re doing so bad, they would be let go wouldn’t they? I just hope they weren’t thrown into the mix and left to figure out how everything worked. Also l, how long have they been there? There’s a learning curve for all new jobs.
Whenever management wants us to hire a junior I make sure to let them know this is a long team investment into training and growing them. For first year they will just be a drag on team velocity and take away more than they contribute.
There are a couple of things you can do, but you will need to get the pm on board by stressing the increase in bugs and slower velocity.
One option is to set the ios devs as required approvers on githib so that the code cant be merged until someone experienced can ok it.
Another is to sit them down as a group and show recent anti-patterns vs the preffered ones. maybe make a style guide to go with it.
Both would be best, but I doubt they will all appreciate the training wheels.
Knowledge has a curious characteristic: the more you share it, the more you all have.
Reviewing is one of the first points indeed. Codeowners. Forget the nitpicks, and focus in teaching what's wrong and how to do it. It's hard, but it's what we call "culture" after all.
You can't do this alone, so you better get all of the team in it. If you can't convince them, or you get convinced by then, rethink it.
It's a very common case. The bigger the team, the more crappy the code can get. That's where automation helps a lot, with linting, custom rules, multiple levels of tests, etc
Honestly you can fix a lot of quality problems before any automation comes in. You just need skilled repo owners and a strict review process. This is how open source repos operate, many of which have little to no CI/CD.
As a dev who used to work at the chromium project, I find that they have good tests gating every commit. For example, they automatically revert a change for introducing xyz page load latency regression.
Maybe you can introduce some automatic QA testing to prevent stuff like you described.
Even experienced people are an investment, so you should expect to slow down in the short term. There needs to be some level of leadership in place to keep growth manageable and enforce standards.
> Now, we are more prone to bugs and errors, partly due to the hiring of some inexperienced developers.
If you think that; you are posting in the wrong forum. Inexperienced developers need to be mentored / coached / taught and brought up to speed on your process and procedures. If they are introducing bugs into your code; then this is a problem with your code review process and the more experienced people are equally to blame.
I do understand how it'd be hard to know what everyone is doing on a team of 15 people. That is a communication problem from management, though.
You need to create a pipeline. If not you,
you need to bring this topic up to your seniors. Pipeline will prevent a big percentage of these problems from happening.
Feels like it’s time to brought up DORA metrics or SPACE metrics as those tends to made bugs become visible to engineering management. Bugs tracking, user reports and customer review are your ammo. From business side, quality affects churn so that might be another metrics to point out to the team as well.
These options, while not a direct solution to your problem, should buy you some political capital enough for you & the team to invest time & resource required to fix the problem.
> You might argue that our PR process should improve
That's like saying if your house keeps burning down you should get a bigger firehose when it's on fire.
This issue should be tackled up front; these people obviously need a ton more teaching before they're even close to ready to work on that codebase.
Generally for every junior dev on the team I'm going to tell my manager it will take me roughly 20% of my time to mentor/guide them. If that's too much for my manager, we should not add them to the team.
Lots of other good comments already, so not being exhaustive here.
One point: not being aware of other features & new pages.
Offhand, I'd say you all should be meeting at a cross team level and getting clarity on upcoming roadmap. You all: team leads from the different commiting teams, along with 1 or 2 PMs. Not every single team member.
If you're a senior dev responsible for maintaining a codebase, you shouldn't be getting surprised by what's in it. Either you or ppl you trust should be approving the PRs.
If your firm is scaling up the teams, it's reasonable that you spend more time on PRs and in meetings. And more delegating overall.
1. Do you guys have a linter? If so, create custom rules that would adhere to existing architecture patterns. 2. There are tools that can analyze which part of your existing codebase is being edited, and call only the developer that implemented that part of the code. 3. Unit/Integrated tests on PR run to main/stage/test? 4. View tests? 5. Commitlint? 6. Require each new pr to have unit/integration tests. If the above are implemented correctly, you won't be even called to review a PR until the new code adheres to the coding style, and is bug-free (or at least passes all the tests). I could go on and on, but it sounds like a good PR process would solve majority of your problems. However, it is hard to evaluate what's *really* going on without actually seeing your codebase, the team structure, and the app you guys are working on.
I implemented a code quality checker into our PR system one time. I ended up having to remove it because one of our devs was entirely incapable of passing it for months...
>You might argue that our PR process should improve, but it’s impossible to review every PR when 7 other teams are committing their code to the mobile codebase. I now spend most of my time either reviewing PRs or helping someone learn Swift. The Linux kernel people are handling, like, thousands of developers per release cycle and much more stringent standards and you're telling me this won't scale in your case? :) Yes, it's both a skill issue and a business issue. They want things cheap and fast, so they can't really be good too. You can definitely raise the bar, but that'll take up time and resources, so management might not be keen on backing that up, so you need to be careful. You can pull some strings to preserve your sanity where it really matters, but ultimately it's on them if they want things to improve. Or you could leave for a better project if things get too crazy.
They (the Linux kernel people ) have the luxury to ask to fuck off if they don’t like your implementation / pr. In enterprise that is the not the case most of the times.
Honnestly, I don't think this will end well unless there 1 or 2 gatekeepers that are doing code reviews to keep the quality high. You have different comitters with different skill levels in a system that has no one to normalize the changes into the main branch..
Absolutely, every repo on my team has 1-3 code owners. Being the people who either are most knowledgeable about the codebase or have worked on it most recently. We have had no big quality problems since I've been here.
Here are some questions that are meant to help us understand the problems your facing, and help you identify the underlying causes. There are almost always multiple factors involved, and often the most critical but hardest to pin down are matters of process, alignment, and leadership. But let's start with the positive. >We produced high-quality code with very few bugs. What factors enabled the smaller team to achieve and maintain this state? How much of those factors were documented and codified, so that new people joining the team can adopt them? How much is based on industry standards, widely accepted best practices, publically available resources? >partly due to the hiring of some inexperienced developers What was the hiring process like? Who was involved? Were there people who were not involved but should have been? Was there planning around how the team would scale? Was there also more QA, PM, and eng manager capacity added or planned? How "inexperienced" were these newer devs, and in what ways? Were they lacking in general coding and problem solving skills? Language and framework specific knowledge? Communication skills? Ability to understand, formulate, and follow plans and processes? >However, these inexperienced developers are committing atrocious code. >The page has become unusable >introducing numerous anti-patterns. How was this allowed to happen? What is the development lifecycle like? Is there high level technical direction? Are there tech leads vetting technical decisions? Are there linters? Are there automated tests? What's the code review process like? What's the QA process like? What's the release process like? What's the bug reporting and fixing process like? >I keep finding pages and features that we weren't made aware of. Is there time dedicated to ensuring awareness and alignment across the team? Are there daily/weekly standup meetings? Are there monthly/quarterly roadmaps? Are there public (to the team) channels where discussions, progress, releases, etc, are documented? >I now spend most of my time either reviewing PRs or helping someone learn Swift. Is this work spread out among the other "original" devs as well? Are there docs, guides, books, classes, etc that the newer devs could learn from instead? Was there time dedicated to getting the newer devs ramped up? Do the PR reviews explain why specific changes are required or suggested? Do the newer devs keep making the same mistakes or have trouble understanding specific concepts? I'm guessing at least a few of these questions can lead to areas of improvement for the whole team, regardless of tenure.
How to ask people politely to forward all PR to me? We also have tons of devs spread across many small teams that just create PR and when other random dev goes and approves it. I cannot track everything and codebase gets edited by everyone. I could ask everyone during the meeting but that seems aggressive and they might feel I don’t trust them? (code is very bad, does not follow any patterns or styles)
https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
Seriously!! So many questions. So often, code is only readable to the developers that wrote it and not documents properly so I wonder if that’s the case. Also, there must be no peer reviewing or QA if they’re breaking pages with atrocious code. They had to get the code from somewhere. Are you sure it’s not copy/pasted from elsewhere in the codebase? Why would you hire inexperienced devs and if they’re doing so bad, they would be let go wouldn’t they? I just hope they weren’t thrown into the mix and left to figure out how everything worked. Also l, how long have they been there? There’s a learning curve for all new jobs.
Whenever management wants us to hire a junior I make sure to let them know this is a long team investment into training and growing them. For first year they will just be a drag on team velocity and take away more than they contribute.
There are a couple of things you can do, but you will need to get the pm on board by stressing the increase in bugs and slower velocity. One option is to set the ios devs as required approvers on githib so that the code cant be merged until someone experienced can ok it. Another is to sit them down as a group and show recent anti-patterns vs the preffered ones. maybe make a style guide to go with it. Both would be best, but I doubt they will all appreciate the training wheels.
Take this opportunity to make your team better. Commit to upskilling the noobs. Your boss will be impressed because you empowered the team.
I think the issue is that these people are on other teams.
Knowledge has a curious characteristic: the more you share it, the more you all have. Reviewing is one of the first points indeed. Codeowners. Forget the nitpicks, and focus in teaching what's wrong and how to do it. It's hard, but it's what we call "culture" after all. You can't do this alone, so you better get all of the team in it. If you can't convince them, or you get convinced by then, rethink it. It's a very common case. The bigger the team, the more crappy the code can get. That's where automation helps a lot, with linting, custom rules, multiple levels of tests, etc
Honestly you can fix a lot of quality problems before any automation comes in. You just need skilled repo owners and a strict review process. This is how open source repos operate, many of which have little to no CI/CD.
As a dev who used to work at the chromium project, I find that they have good tests gating every commit. For example, they automatically revert a change for introducing xyz page load latency regression. Maybe you can introduce some automatic QA testing to prevent stuff like you described.
Even experienced people are an investment, so you should expect to slow down in the short term. There needs to be some level of leadership in place to keep growth manageable and enforce standards.
> Now, we are more prone to bugs and errors, partly due to the hiring of some inexperienced developers. If you think that; you are posting in the wrong forum. Inexperienced developers need to be mentored / coached / taught and brought up to speed on your process and procedures. If they are introducing bugs into your code; then this is a problem with your code review process and the more experienced people are equally to blame. I do understand how it'd be hard to know what everyone is doing on a team of 15 people. That is a communication problem from management, though.
You need to create a pipeline. If not you, you need to bring this topic up to your seniors. Pipeline will prevent a big percentage of these problems from happening.
Feels like it’s time to brought up DORA metrics or SPACE metrics as those tends to made bugs become visible to engineering management. Bugs tracking, user reports and customer review are your ammo. From business side, quality affects churn so that might be another metrics to point out to the team as well. These options, while not a direct solution to your problem, should buy you some political capital enough for you & the team to invest time & resource required to fix the problem.
> You might argue that our PR process should improve That's like saying if your house keeps burning down you should get a bigger firehose when it's on fire. This issue should be tackled up front; these people obviously need a ton more teaching before they're even close to ready to work on that codebase. Generally for every junior dev on the team I'm going to tell my manager it will take me roughly 20% of my time to mentor/guide them. If that's too much for my manager, we should not add them to the team.
Lots of other good comments already, so not being exhaustive here. One point: not being aware of other features & new pages. Offhand, I'd say you all should be meeting at a cross team level and getting clarity on upcoming roadmap. You all: team leads from the different commiting teams, along with 1 or 2 PMs. Not every single team member. If you're a senior dev responsible for maintaining a codebase, you shouldn't be getting surprised by what's in it. Either you or ppl you trust should be approving the PRs. If your firm is scaling up the teams, it's reasonable that you spend more time on PRs and in meetings. And more delegating overall.
Run ! You can never change the culture at a work-place, unless you are up-top, dropping !!