Azure
tooling: removing the auto-close bot
This commit fixes #19098 by removing the auto-close bot.
Whilst I can understand that Service Teams maybe unable to review all Pull Requests in a timely manner, unfortunately I've had a number of Pull Requests at this point which end up with me posting every 2 weeks to keep the Pull Request open:
- https://github.com/Azure/azure-rest-api-specs/pull/17987#issuecomment-1066131357
- https://github.com/Azure/azure-rest-api-specs/pull/18269#issuecomment-1086897732
- https://github.com/Azure/azure-rest-api-specs/pull/20961#issuecomment-1312764448
- https://github.com/Azure/azure-rest-api-specs/pull/21291#issuecomment-1312764141
Whilst this does mean there'll be more Pull Requests open until the Service Team can take a look at them - removing this user hostile bot improves the contribution experience.
Hi, @tombuildsstuff Thanks for your PR. I am workflow bot for review process. Here are some small tips.
![openapi-workflow-bot[bot] avatar](https://avatars.githubusercontent.com/in/81125?v=4 "Published by a pub.dev verified publisher"){kind=small}
Swagger Validation Report
️️✔️BreakingChange succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️️✔️LintDiff succeeded [Detail] [Expand]
Validation passes for LintDiff.
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️ApiReadinessCheck succeeded [Detail] [Expand]
️️✔️~[Staging] ServiceAPIReadinessTest succeeded [Detail] [Expand]
Validation passes for ServiceAPIReadinessTest.
️️✔️~[Staging] SwaggerAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️CadlValidation succeeded [Detail] [Expand]
Validation passes for CadlValidation.
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
![openapi-pipeline-app[bot] avatar](https://avatars.githubusercontent.com/in/65541?v=4 "Published by a pub.dev verified publisher"){kind=small}
Swagger Generation Artifacts
️️✔️ApiDocPreview succeeded [Detail] [Expand]
️️✔️SDK Breaking Change Tracking succeeded [Detail] [Expand]
Breaking Changes Tracking
Thank you for your contribution tombuildsstuff! We will review the pull request and get back to you soon.
Swagger pipeline started successfully. If there is ApiView generated, it will be updated in this comment.
Hi, @tombuildsstuff Thanks for your contribution. Are you from azure or open source community? This changes may have some impact on our review process. Could you provide more background information here?
@ruowan see #19098 for the background - Service Team’s aren’t reviewing or responding to PR’s before the bot fires and it’s impacting/exerting effort for OSS contributors (who have to comment every 2 weeks until the Service Team get a chance to review it), which has a knock on effect to all consumers of the services - as such this PR looks to save everybody unnecessary notifications
Happy to take an alternate suggestion if there’s a faster way to get Service Team’s to review PR’s though? :)
I understand. The core issue is service team doesn't review PR activitly not the bot automation behavior. But the bot ask oss contributor to active PR every two weeks. It's not user-friendly for oss contriubtor. I think maybe there I two pential solutions.
- Add label filter for bot. Don't auto close PR when PR labelled with customer-reported.
- Find a faster way to get service team to review PR.
How do you think about it?
For second part, we need to discuss with our internal team.
@ruowan perhaps the bot could be updated to ping the Service Team every 2 weeks instead, or a label as you've suggested to allow a team to manually work through the list and ping the teams through another channel (to do the same thing) - rather than trying to close the PR?
To be honest I don't think there should be a difference between internal and external PR's here, else it's easy for the external ones to get ignored with a separate process? Ultimately we're all working towards the same goal of trying to ensure the Swagger data is correct so that folks can make use of it/the services :)
I helped ping service team on teams channel. We will sync up with our PR assignee to know and help route such customer contributed PR.
@akning-ms
Based on Ruoxuan's comment, Should we close this PR. @tombuildsstuff
Whilst I appreciate that @ruowan has been running a few of the PR's down, unfortunately this PR is still necessary since the auto-close bot is causing significant amount of work for third-party contributors, for example:
As such, what's needed to merge this PR?
@tombuildsstuff I created an alternative PR. With this change, our bot will not mark the customer-reported PR as stale. https://github.com/Azure/azure-rest-api-specs/pull/21837
Hi @tombuildsstuff, Since Ruoxuan have new PR to fix your issue in PR#21837, can you confirm & close this PR ?
@akning-ms unfortunately this issue isn't fixed, see the latest update in #19098
@tombuildsstuff I have applied fix. Bot will check whether the PR has customer-reported label. And for customer-reported PRs, our bot won't close them.
@tombuildsstuff I have applied fix. Bot will check whether the PR has customer-reported label. And for customer-reported PRs, our bot won't close them.
And will also not give any comment to reminder any more for new or old PR as above screenshot Tom pasted in above, right?
@ruowan I've seen a number of fixes closed over the years from both external contributors and Microsoft employees from this - so unfortunately this also affects PR's without the customer-reported label, so I think that needs to be taken into account too?
@ruowan I've seen a number of fixes closed over the years from both external contributors and Microsoft employees from this - so unfortunately this also affects PR's without the
customer-reportedlabel, so I think that needs to be taken into account too?
Hi @tombuildsstuff, this is by design to improve review efficiency as PR assignee have to spend some time monitor these inactive PRs. for PR author, it is also easy to reopen it if the PR is still need. but you are right, this may not fit for the PRs(customer-reported), that is why Ruoxuan changed the bot setting to ignore this kind of PRs.
Hi @tombuildsstuff, I am closing it for now as not get your further response, pls free to reopen if need.
@akning-ms @ruowan here's 3 more PRs, this time from Microsoft employees that are valid and fix real issues in the API (by updating the Swagger to reflect the API behaviour) that have been closed by the bot as the Service Team hasn't responded within the SLA:
https://github.com/Azure/azure-rest-api-specs/pull/27524 https://github.com/Azure/azure-rest-api-specs/pull/25435 https://github.com/Azure/azure-rest-api-specs/pull/25142
Whilst I agree with you that the underlying issue here is that Service Team should be more responsive so these don't get auto-closed - they're not - so removing this behaviour would be a significant quality of life improvement versus just closing these out.
I can see that this behaviour now lives in another file, so if it's helpful I'm happy to open a new PR to fix that - but can you confirm when this problematic behaviour will be removed?
Thanks!