Terraform-provider-azurerm: Pull Request reviews
Is there anyone actively reviewing XXL pull requests on the team right now?
I've been trying to get a PR approved for just over two weeks now but nobody seems to be actively reviewing them.
In the meantime, i've seen many XS, S, and M's approved during that time period.
I get low hanging fruit, but people also want to contribute and add new features to the provider, and are unable to, seemingly, due to a lack of desire to review larger PR's.
Is anyone able to comment on why this might be?
A potential solution is to allow more people to review PR's, if it's a time/people bottleneck.
IanMoroney
All 2 comments
Hi @IanMoroney
Thanks for the question here.
I'll try to cover as much as I can.
The size of tickets isn't generally a factor in of itself, but the impact of the changes involved can be. The PR to which you refer is an XXL, but the challenge with it is in updating the API version in use against a fairly sizeable number of resources. In the past such things have caused unexpected, and indeed undesirable side effects, so we try to be as cautious as is reasonable with them. I have a partial review already completed which I started when I labelled it last week. However, this is time consuming and the team has demands on it outside the immediate open source project, from our own customers and from Azure teams, these activities can be derailed for a time. Since I started on the review, and the team try not to work at cross purposes, this has ended up in a delay on your PR while I picked up a piece of work for a customer requirement that was larger than expected.
The other PR's you will have seen moving through will have had various drivers, some will be as you say, low hanging fruit, that one of the team felt they had time to include/review/test in between other tasks, or on a push for clearing down the PR count so we can have better visibility. Others will have been for fixes to failures, resolutions to behavioural changes in APIs / services, or other features that are required.
Ultimately, we try to spread our effort evenhandedly, but we are working with a number of external factors that change our planned effort regularly. We highly value community contribution such as yours, and we try to review them as promptly / quickly as we can.
I hope this helps explain the situation?
I'm going to close this issue since there's opportunity to discuss the PR this relates to on the PR itself, for which I'll be picking up the review of again today.
jackofallops
on 6 Aug 2020
I'm going to lock this issue because it has been closed for _30 days_ ⏳. This helps our maintainers find and focus on the active issues.
If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!
![hashibot[bot] picture](https://avatars2.githubusercontent.com/in/8332?v=4&s=40)
hashibot[bot]
on 5 Sep 2020
Related issues
mikemorris
·
3Comments
tomasaschan
·
3Comments
voyera
·
3Comments
Lachlan-White
·
3Comments
mlazowik
·
3Comments
Most helpful comment
Hi @IanMoroney
Thanks for the question here.
I'll try to cover as much as I can.
The size of tickets isn't generally a factor in of itself, but the impact of the changes involved can be. The PR to which you refer is an XXL, but the challenge with it is in updating the API version in use against a fairly sizeable number of resources. In the past such things have caused unexpected, and indeed undesirable side effects, so we try to be as cautious as is reasonable with them. I have a partial review already completed which I started when I labelled it last week. However, this is time consuming and the team has demands on it outside the immediate open source project, from our own customers and from Azure teams, these activities can be derailed for a time. Since I started on the review, and the team try not to work at cross purposes, this has ended up in a delay on your PR while I picked up a piece of work for a customer requirement that was larger than expected.
The other PR's you will have seen moving through will have had various drivers, some will be as you say, low hanging fruit, that one of the team felt they had time to include/review/test in between other tasks, or on a push for clearing down the PR count so we can have better visibility. Others will have been for fixes to failures, resolutions to behavioural changes in APIs / services, or other features that are required.
Ultimately, we try to spread our effort evenhandedly, but we are working with a number of external factors that change our planned effort regularly. We highly value community contribution such as yours, and we try to review them as promptly / quickly as we can.
I hope this helps explain the situation?
I'm going to close this issue since there's opportunity to discuss the PR this relates to on the PR itself, for which I'll be picking up the review of again today.