New pattern suggestion: Managing capacity for reviewing contributions #692
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| ## Title | ||
|
|
||
| Managing capacity for reviewing contributions | ||
|
|
||
| ## Patlet | ||
|
|
||
| Reviewing InnerSource contributions takes time and effort. This should be reflected in capacity planning, especially for larger contributions. Expectations and available capacity should be transparent so that contributors understand when their contributions will be reviewed and, if accepted, released. | ||
|
|
||
| ## Problem | ||
|
|
||
| Large InnerSource contributions are causing delays to other work in the team and/or contributions are taking longer to be released than expected. Reviewing contributions may be significant invisible work, not tracked in a team's Agile development process. | ||
|
|
||
| ## Story | ||
|
|
||
| The BBC's connected TV application are built by a number of teams, each with different areas of responsibility. They work on each other's areas of the codebase via InnerSource on a regular basis. | ||
|
|
||
| The team responsible for the build process for the JavaScript bundles received a major pull request, changing how dependencies were bundled. This PR introduced a new build time dependency, a new structure to the deployed JavaScript bundles, and touched 503 files, with 6,699 lines of code added and 2,828 lines of code deleted. A contribution of this size required significant time to code review, test, and ensure the team understood the new tooling and structure introduced. | ||
|
|
||
| Normally, InnerSource contributions would be reviewed ad-hoc, but the team estimated that this review process would take days rather than hours. Reviewing this PR would have delayed the team's other work, so the team raised this with the team lead, project manager, and product manager, to prioritize this work against other work. Time was set aside to review this contribution at a future date. | ||
|
|
||
| This process was formalized in the team: | ||
|
|
||
| * Larger contributions have a ticket created on the team's backlog and included in prioritization discussion alongside other tickets. The contributor will be informed of the priority call and given an estimate as to when it will be reviewed and released. | ||
spier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * Smaller contributions can still be reviewed ad-hoc. | ||
|
|
||
| ## Context | ||
|
|
||
| Host team of a successful InnerSource project are finding it difficult to review contributions, especially large contributions. They do not want to disrupt their team's other work, but also want to support contributions by reviewing/releasing them in a timely fashion. | ||
|
|
||
| ## Forces | ||
|
|
||
| * Contributors expect timely feedback on their contributions | ||
| * Host team are expected to deliver other work alongside reviewing contributions | ||
| * Missing communication between contributors and host team on expectations/lead time for contributions to be reviewed/released | ||
| * Tension in prioritizing InnerSource contributions against other work | ||
| * Contributors already strive to make small PRs in line with Agile, InnerSource, and Continuous Delivery principles, but find instances where larger PRs are unavoidable | ||
|
|
||
| ## Solutions | ||
|
There was a problem hiding this comment. It might be a good idea to recommend that the contributing team informs the receiving team as early as possible when planning large contributions. One form to operationalize this is through Draft PRs, which allows early feedback, specially if the PR text contain a summary of the planned changes. Using stacked PRs and slicing the reviews into smaller parts could also help. There was a problem hiding this comment. What I have seen many host teams do is to ask for an issue before creating a PR. Or in other words, starting the conversation about the why and what (in an issue) before sending the how (in a PR). We could add a note about this here, if it fits. And I am even starting to think that this point could work as a new tiny pattern as well a la "Remember to talk to each other before sending code" :) There was a problem hiding this comment. @dellagustin-sap with "stacked PRs" you mean breaking up a larger contribution into multiple PRs that should be reviewed in a specific sequence? i.e. first PR1, then PR2, then PR3, ...? There was a problem hiding this comment. @spier , I'm not sure that there is a well understood definition for the term "stacked PRs", but what I meant was something like:
This only works of course if the change can be split into small increments. That's not always the case. There was a problem hiding this comment.
Yes I will add this. There are a few methods I think worth mentioning: drafts PRs (per @dellagustin-sap), issues (per @spier), RFCs, or even an informal conversation e.g. in Slack
Definitely agree this is preferable but goes a little beyond the scope of this pattern. Small increments is IMO best practice for Agile, InnerSource, CD... Perhaps I'll mention this in 'forces' - something like 'contributors already strive to make small PRs, but find instances where larger PRs are unavoidable.' There was a problem hiding this comment. Amazing changes @tsadler1988! @dellagustin-sap as you started this particular thread, would you mind giving this a review? And while you are at it, possibly even leaving an official review on the PR, to confirm that I am not the only one approving it. Cheers :) There was a problem hiding this comment. IIRC we even mention the "please talk to the receiving team before submitting a PR" part in the learning path :) It's one of the things that I keep telling people even in open source: Don't send a PR as your first line of communication. Nothing is more frustrating than going though learning the project, making the modification work and then being told that the receiving team does not accept it - no matter how good the reason may be. |
||
|
|
||
| * Contributors are encouraged to give the host team early visibility of larger contributions (e.g. via GitHub Issues, draft PRs, [RFCs](../2-structured/transparent-cross-team-decision-making-using-rfcs.md), or informal discussions) and are made aware that not doing so could delay review of their contribution | ||
| * Reviewing larger contributions is tracked in the team's ticketing system/bug tracker (e.g. Jira, GitHub issues) | ||
| * Host team is given time to review larger contributions in team capacity planning | ||
| * Reviewing contributions is prioritized against other work (e.g. in sprint planning) | ||
| * Host team communicate their capacity for reviewing contributions, the priority of contributions, and an estimate of when a contribution will be reviewed/released | ||
| * Host team has a service level objective (SLO) (e.g. 2 working days) for contributions receiving initial feedback | ||
| * Smaller contributions are still reviewed ad-hoc; the team may have guidelines on what they consider to be a smaller contribution (e.g. review should take less than an hour) | ||
|
|
||
| ## Resulting Context | ||
|
|
||
| Host 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 host team will provide initial feedback. | ||
|
|
||
| Smaller PRs are still reviewed ad-hoc, minimising overhead and unnecessary additional process. | ||
|
|
||
| There may still be conflict in prioritising contribution reviews, especially if the host team is overburdened with other work. This only works if contributions are valued by the decision makers in the team's planning process. | ||
|
|
||
| ## Known Instances | ||
|
|
||
| BBC iPlayer & Sounds | ||
|
|
||
| ## Status | ||
|
|
||
| Initial | ||
|
|
||
| ## Author(s) | ||
|
|
||
| Tom Sadler | ||
|
|
||
| ## Acknowledgments | ||
|
|
||
| Clare Dillon | ||
| Sebastian Spier | ||
| Guilherme Dellagustin | ||
| Michael Basil | ||
| Bill Westfall | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor comment. Typically in patterns, the story is generalized to not mention a specific company or institution (it's a pattern, so it ideally should be described in ways that can be applied to other companies/users). So, maybe:
An application developed by a company for use by users outside the company is built by a number of teams, each with different areas of responsibility.
The specific information becomes apparent in the known instances reference.