Document policies regarding draft/wip and staled Pull Requests #56523
Conversation
Add: - Policy about WIP and draft PRs being unacceptable on the repo - Policy regarding staled PRs
There was a problem hiding this comment.
I'm OK with the proposed policy. PR should be in a finished state when proposed.
We could also point out that if a developer want to discuss about a current work in progress, the best way to do it is to send a link to a branch/fork on the developer mailing list.
| ## Policies | ||
|
|
||
| - "Draft" and "WIP" pull requests are **not** accepted on this repository. Please get your work to a stage where **you** consider it ready for | ||
| merge prior to opening a pull request. The one exception to this rule is opening a short term, not-for-merge PR in order to conduct a full CI test run on the branch. Please make sure you close the pull request ASAP after conducting this test in order to keep the QGIS pull request queue clean and manageable. |
There was a problem hiding this comment.
Actually, it's possible to trigger the CI by just adding the branch name of your fork here, so I would be in favor of no exceptions
There was a problem hiding this comment.
@troopa81 are you sure? We have particular conditions for jobs running under QGIS GH account, if the same workflow is run in the user's account it will certainly fail (timeouts).
There was a problem hiding this comment.
That means running this on your own fork which comes with additional configuration effort (which might hard to find for people not used to the github ecosystem)?
There was a problem hiding this comment.
@troopa81 are you sure? We have particular conditions for jobs running under QGIS GH account, if the same workflow is run in the user's account it will certainly fail (timeouts).
It was still working a few weeks ago. It's my usual workflow when I want to try a modification that have great chance to break things (this one was all green for instance)
That means running this on your own fork which comes with additional configuration effort (which might hard to find for people not used to the github ecosystem)?
Only this one no?
There was a problem hiding this comment.
If you know which buttons to press and files to change it's "easy" (but there's still the cold cache which might time out according to @elpaso)
But I am not convinced the "win" for us is big enough to justify raising the entry barrier with this and the extra effort for the review team to also monitor a mailing list.
|
I'm not convinced that we need a hard rule, unless there is a change in the attention that developers dedicate to the developers ML I think that posting a link to a branch to the ML is non going to attract the same attention of a draft PR. I agree that the main repository should not be confused with a personal staging area, but I think that WIP and Draft PRs (which are in my view the same thing) should be used (and not abused) sparingly when asking for early feedback on an implementation/refactoring or a prototype which is not big enough to deserve a QEP (or does not involve new features or API changes). I'd leave the stale bot in action as an extra measure in order to ensure that these drafts do not stay open forever. |
|
I do think we can make use of draft PR's without treating them as something precious to protect from the stale bot. For instance, if someone who is not a hardcore committer (especially people who are not yet very well used to all the subtle rules and technicalities of the QGIS repository), once a PR is opened and a first review done, it could be converted to "draft" and the author asked to mark it ready for review when they are done with the reaction. I find that a much nicer way than giving a big red "request changes". During this follow up work I don't mind keeping it open for a couple of days, but it should still stale away if no action follows. |
|
For me the second part about stale PRs would be sufficient, so I would just drop the first part about draft and WIP PRs |
|
@m-kuhn would we also need some explicit text stating "don't just post 'not stale'" as a way of avoiding stale bot? Otherwise there's nothing explicitly stating that this kind of behaviour is in violation of the repository policies. |
|
@nyalldawson as long as it's explicitly aimed at unfinished PRs, that's ok, yes. I.e. PRs that are ready for review and just don't get the attention they deserve should use keep alive (e.g. #55172 is a perfect example of something that should not run any risk of being collaterally damaged). |
|
The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check
|
github-actions
bot
added
the
stale
|
While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist. |
Add:
Alternative to #56062