Replies: 4 comments 2 replies
-
This is, I believe, the issue with “staff duty”. PRs are inherently async, so discussions can actually be hindered by someone sitting around waiting for someone else to comment on their comment on a PR. And I know we talked about this being a role in addition to engineering work, which then begs the question…why are we expecting this standard level of operation from ourselves only when we’re assigned this role for a sprint? That’s where this begins to show the need for all of us to embrace this change in PR ops, not assign it to a role and pass it around like a hot potato. That approach, in fact, makes me think we don’t value our PR ops (along with our lack of labeling, assigning, safety settings, etc.) enough to qualify it as part of all of our jobs.
To this point, in the same vein as “this is all our responsibilities, not just the responsibility of a role-holder”, you’re relying on a single person to lead a healthy PR culture for two straight weeks, whereas a cultural change to make better PR ops our baseline of operation would enable us all to support one another day-to-day with the struggles you acknowledge. |
Beta Was this translation helpful? Give feedback.
-
Additional context ported from Slack: I totally feel the need for more attention paid to PRs and our non-verbal communication (i.e. commit messages and diffs, PR descriptions, etc.), but I think appointing a person per sprint to be the leader in that could deter the rest of us from feeling responsible for building a better PR and engineering culture. I want us to be accountable to ourselves and each other when it comes to making these improvements, and sprint-to-sprint assignment of it means it’ll only truly be “my job” once a PI. I’ll attach the “Git Good Practices” guidance from Fearless. Particularly, within the Resources section, they call out a few good sources for writing better descriptions. I’ll also see if I can get any documentation from David Gage at Fearless about updated PR methodologies (I think this document is a year old, prior to his job existing). Him and I have talked about principles like “always write two drafts” where your first round of reviews never results in an approval, and you should always self-review and refactor before you consider a second review. Things like this, I believe, build that PR culture we’re hoping for. Gage had a few good questions for us to ask ourselves:
|
Beta Was this translation helpful? Give feedback.
-
Update on this, @pkim-gswell and I chatted last Friday about our ideologies and we came to a conclusion that a joint approach would help tackle the immediacy of the need while building a culture around it that perpetuated. Two key things are necessary for the approach to be effective:
|
Beta Was this translation helpful? Give feedback.
-
Noting: We talked as a group yesterday and are going to trial this position this sprint. @pkim-gswell has a first draft of the role's responsibilities/guiding principles; that will be used for this sprint and refined. The plan is to regroup on this during retro. |
Beta Was this translation helpful? Give feedback.
-
Idea Summary
A developer designated on a per-sprint basis whose primary duties are PR discussion facilitation, code promotion through higher environments, and possibly other things that make sense to collate. We've so far referred to this role as staff duty.
Detail
An idea was raised to have a designated developer be on point for pull requests for a sprint. So, pull requests are assigned to that developer, and that devs primary and first duty is to start and facilitate the PR process.
That developer jumps on PRs when they’re marked ready for review, and really goes through them carefully with a critical eye. Along with adding his/her own review, that dev should rope in (tag) other developers as they deem appropriate. So if maybe there’s a question about zod best practices... tagging brian would make sense.
This isn’t meant to allow the other developers to ignore pull requests; please review as you have time. But the hope is that a designated person whose primary responsibility is timely and quality review, facilitating discussion, and raising concerns early will lead to a better product and a more predictable PR experience.
Another area this can alleviate is knowing whose go ahead you need to merge. I’ll raise as a good rule to be: “you need the staff duty person’s approval to merge”. If that dev sees that all other discussions are resolved, and there’s consensus this is work to merge, that dev should approve. That should represent the team’s/product’s approval.
Also proposed: this staff duty person might be best positioned to promote code to val and production as appropriate, along with reindexing those environments as needed. Historically we’ve not promoted to val/prod in a timely manner, for a few reasons. There’s not much external pressure to promote yet, stories need to be validated in master, and sometimes it may be wise to hold a feature or two in master for something else about to merge. Considering this staff duty person will be closely involved with all PRs for a sprint, we should consider adding code promotion to their responsibilities.
It’s proposed that this staff duty person’s primary responsibility is PR and promotion facilitation (and/or whatever else we decide as a group), but that’s likely not a full time job. So, work can still be assigned to this person, but not a full plate. It’s suggested we make a ticket per sprint that covers this staff duty role, and assign it points accordingly, to track the time/work spent.
Let’s talk about this, zero in on what responsibilities should be, and we’ll codify it.
Beta Was this translation helpful? Give feedback.
All reactions