Conversation
|
lgtm |
pull_request_template.md
Outdated
| @@ -0,0 +1,33 @@ | |||
| ## 🎫 Ticket | |||
|
|
|||
| Link to the relevant jira ticket. | |||
There was a problem hiding this comment.
| Link to the relevant jira ticket. | |
| Link to the relevant Jira ticket. |
It's a proper noun?
There was a problem hiding this comment.
or drop Jira entirely, and just say "ticket"?
| Link to the relevant jira ticket. | |
| Link to the relevant ticket. |
There was a problem hiding this comment.
I'll drop it entirely.
|
FYSA, I am going to merge this once all checks pass. In the spirit of having a month-long trial period for this new process, I will bring up the PR template during our October 26 engineering huddle. (I don't think it's possible to put something on the agenda that far in advance, but I'll set a calendar and slack reminder to remind myself of the topic.) |
|
|
||
| </details> | ||
|
|
||
| ## 🚀 Notes for Deployment |
There was a problem hiding this comment.
Who is the intended audience of these notes? The deployer?
There was a problem hiding this comment.
Yes, I was thinking of scenarios where we might need to ensure that two PRs are deployed at the same time, or that one PR is deployed first. I saw the first scenario on a previous project. I was imagining it as a possible scenario on this project, too, but let me know if you think this section isn't relevant.
There was a problem hiding this comment.
I'd wonder how the deployer is to be made aware of these notes. Since there's nothing in the deployment guide to set the expectation that a deployer is to be reviewing each pull request for these notes, I don't think we can expect that they would.
For the scenarios you mentioned, those seem to me as being the responsibility of the code author to ensure any dependencies are merged when appropriate based on the regular deployment cadence, since those sorts of selective deployments would not be possible with the current process.
That being said, we could always revisit the steps of the deployment process itself, but I'd lean toward avoiding extra work for the deployer if we can delegate that to the individual contributors.
No need to act on it now if we'd want to reserve this kind of feedback for the planned follow-on discussion. 👍
There was a problem hiding this comment.
I agree, I'd guess the vast majority of PRs won't have unique notes for deployment and we could potentially drop this section.
There was a problem hiding this comment.
Yeah, I do agree with avoiding extra work for the deployer. Asking the deployer to review each PR’s “Notes for deployment” section would obviously make their job harder. Also, in the example I gave, the code author could use stacked PRs.
The follow-up discussion is October 26 in engineering huddle. Are you both fine with leaving the PR template as is until then?
There was a problem hiding this comment.
The follow-up discussion is October 26 in engineering huddle. Are you both fine with leaving the PR template as is until then?
Totally fine with me 👍
6 participants
🎫 Ticket
Create a PR Template for the identity-idp repo
🛠 Summary of changes
📜 Testing Plan
Provide a checklist of steps to confirm the changes.
👀 Screenshots
n/a
🚀 Notes for Deployment
n/a