[Pattern Draft] Managing capacity for reviewing contributions #692
Conversation
tsadler1988
requested review from
lenucksi,
NewMexicoKid,
cewilliams,
spier,
robtuley and
yuhattor
as code owners
spier
added
1-initial
There was a problem hiding this comment.
Nice one @tsadler1988!
I have seen something similar at work as well, at least in the sense that the act of "reviewing contributions" is being tracked as "real work".
I might have some more practical questions about how you do these things in practice but already wanted to provide a quick review to get the process started.
There was a problem hiding this comment.
@tsadler1988 thank you for sharing this pattern with us.
What I appreciate most about it is the focus on a specific, seemingly minor topic that can significantly impact how "InnerSource collaboration" is perceived within the organization.
In the worst-case scenario, someone might complain, "Oh, I hate these InnerSource contributions; they always disrupt my team's delivery predictability." While I don't agree with that view, if this pattern can mitigate that risk by treating "review time" as actual work, then I'm all for it!
One thing I'm curious about:
Does your team allocate any slack time when planning sprints? That is, do you leave time available for "unplannable work" that inevitably arises? I usually try to keep 10-30% slack time for teams, allowing them to react more flexibly to what happens during the sprint. It might seem like a lot, but it has worked very well for me to keep teams happy and productive.
Overall, I'd be happy to merge the pattern as it is or with minor changes. Once merged as an initial pattern, we can share it more easily with others to gather further "known instances."
Later, we can add visuals and other enhancements to give this pattern more recognition and prominence once it's published in the online book 🥳
Thanks again Tom!
Co-authored-by: Sebastian Spier <github@spier.hu>
| * Maintaining team is given time to review larger contributions in team capacity planning | ||
| * Reviewing contributions is prioritized against other work (e.g. in sprint planning) | ||
| * Maintainers communicate their capacity for reviewing contributions, the priority of contributions, and an estimate of when a contribution will be reviewed/released | ||
| * Maintaining team has a service level objective (SLO) (e.g. 2 working days) for contributions receiving initial feedback |
There was a problem hiding this comment.
I love the SLO because it creates productive back pressure, transparency and utility for the development team, product owner, scrum master and project management personnel.
It also layers into moving in alignment with DevOps / SRE / Observability approach.
There was a problem hiding this comment.
+1 on the SLOs - not only does that create back pressure - it also helps communicate to contributors what expectations can be wrt. to response times. Especially inside of companies those can vary greatly between teams.
Btw. what I found helpful for host teams is to track "time to initial response" - https://github.com/github/issue-metrics (I found that metric way more helpful than something like time to close - time to first response is something you can easily optimise. Time to close not so much because minimising that tends to have nasty side effects.)
|
@tsadler1988 - I reviewed this with my colleague Bill Westfall (who I introduced you to in Slack). I left a couple of comments, but overall I love the pattern. Great to see this drafted cleanly. Kudos. |
|
|
||
| ## Resulting Context | ||
|
|
||
| Maintaining team understands the overhead of reviewing large contributions and is given capacity to do so. Project manager and product managers are better able to plan, estimate, and track other work in the team by accounting for the time taken to review InnerSource contributions. Contributors understand when their contribution will be reviewed and released, and how long before the maintainers will provide initial feedback. |
There was a problem hiding this comment.
Agree that product and project managers are the best people to filter this before a large PR hits the team unaware. Would be curious to chat and understand the intake process better and what part of that may/may not be working right now.
There was a problem hiding this comment.
Yeah I'm making an assumption that there's a foundation of contributions being valued, and even if they are it could be difficult to prioritise them against other work. Some teams may go to the extreme of having a policy of 'merge every contribution' whereas others will be pickier.
It might also be good to specifically call out that smaller PRs are still dealt with ad-hoc - IMO we wouldn't want smaller PRs being blocked by bureaucracy.
There was a problem hiding this comment.
That matches my observations as well: Small changes tend to be smooth. It is larger changes, either as one PR or multiple ones that are related that cause tension.
Good question @spier. When we did Kanban we would just prioritise contributions as they came in. I'm less directly involved with the dev teams these days, but we've recently moved to Sprints so I will ask around. |
|
Looks like this might be ready to be merged rather soonish. Awesome! |
Looks like we continue to be flexible like we were in Kanban, rather than building in slack. 'Asteroids' as we call them (live issues, InnerSource contributions etc.) might be prioritised during the Sprint, bumping other items originally included in the Sprint. So the Sprint planning is more of a 'this is what we hope to deliver in this Sprint assuming no asteroids'. Perhaps this is a luxury of building software internally - this approach might not fly when building for a customer! |
|
This is always a tension that develops with teams that are successful to get customers. I have some approaches I use with Agile to start out strict Scrum as a training mechanism at formation and or during stretches when the team is really squishy. Happy to unpack a conversation around this sometime in a call. Can be brought into Dojo Circle on Friday at 13:00 UTC see #dojo-circle Not every conversation can be maximized asynchronously! |
I fully agree. I would like to add that this is not only true for InnerSource but also something that I have seen in open source: I have seen, often single vendor, open source projects that were trying to increase committer diversity to go beyond the founding company. For that to happen, there needs to be time to review PRs that are not coming from your own company. Often though that clashes with how developers are incentivised: Reviews may not be something that counts for performance reviews. Reviews of PRs that do not pay into the current tech strategy may not be something that counts. If there is time pressure often those things win that provide a short term benefit for one's own business goals wins. Making mentoring time transparent and something that is explicitly tracked can help with raising awareness for this work. It can help teams understand that this is what helps engineers gain impact beyond their own team and grow as a result. Tracking the number of very active reviewers with metrics can also help raise questions and increase that understanding. |
|
I really like having that here as a pattern - it's something that I have been explaining to teams within InnerSource but also in open source over and over again. I've forwarded the draft pattern directly to our InnerSource teams as it does a really good job at providing a well phrased, easy to understand description. |
@tsadler1988 I love the term Asteroids for this :) So "Handling Contribution Asteroids" could a be a fun title for this as well then :) Or "Oh no, somebody wants to help, what do I do?" :) I have cleaned up all open threads on this pattern that I could find. Will let this sit here for a couple of days. If there is no major disagreement, I will then merge it . As this is still an initial pattern it is good for us to get this integrated into our repo, so that we can invite further contributions by the community. I would expect that many companies will have implemented this pattern in some shape or form. Wondering what different names this idea goes by. Maybe others are also calling these things Asteroids, who knows :) |
There was a problem hiding this comment.
Ready to be merged, when @tsadler1988 is happy with the latest version.
Lazy consensus on 5.1.2025 (i.e. without feedback I would merge then)
spier
changed the title
|
Thanks all for the input on this, especially @spier for driving this through to conclusion.
I'm torn between making pattern titles dry but descriptive vs more engaging but less clear. Happy to go with whatever we think would generate more views!
I've addressed @NewMexicoKid's feedback as a commit suggestion and a question on potential future improvement, but otherwise yes I think we can merge. |
Co-authored-by: Tom Sadler <thomas.sadler@bbc.co.uk>
|
@tsadler1988 I say: Let's get this merged, and try to find more orgs with similar concepts. That will help us to improve the pattern further. Thank you for your contribution! |
No description provided.