+1
Sent from Gmail Mobile On Wed, May 13, 2026 at 3:03 PM Adam Monsen <[email protected]> wrote: > I was working on improving our branch protections > <https://issues.apache.org/jira/browse/FINERACT-2605> today and was > forced to change our workflow a bit. > > *Before*: commits directly to develop were allowed. If a PR is used, all > conversations in that PR must be resolved. > > *After*: commits directly to develop are not allowed. All changes must > come from approved PRs, and all conversations in PRs must be resolved. > > What do people think of this change? Not an official vote but: +1? -1? > Must all changes come from approved PRs? Note another side effect: *Someone > else* must review and approve your PR. For example, PR #5846 > <https://github.com/apache/fineract/pull/5846>. > > Personally I'm +1 on .asf.yaml as it is now > <https://github.com/apache/fineract/blob/e3d698bc1f2d297ad124e10c2fbf2596f91bb00b/.asf.yaml>, > enforcing the "After" rule as stated above. > > I think it's a reasonable set of rules to enforce since AFAIK we already > follow this in practice, but there may be edge cases I'm not thinking of. > When I browsed git commit history the only exceptions I found were my own > commits directly to develop. > > If we decide we don't want the new rules, we'll need to remove these lines > from .asf.yaml: > > required_conversation_resolution: true > required_pull_request_reviews: > required_approving_review_count: 1 > > I wanted only the first line to keep things the way they are, but using > rulesets I can't have it without the other two lines. And we must now use > rulesets. So I thought through the new way (requiring at least one > approved PR), and I think that'll probably be a Good Thing. So I'm still +1. > > (Not to complicate things too much but I believe we can omit the > conversation resolution requirement and keep the approved reviews > requirement. So yet another option. I prefer to keep the conversation > resolution requirement.) > > Feedback welcome. If nobody has any strong feelings about this I'll just > use lazy consensus and leave it as-is. > > -- > Adam Monsen > Software Engineer » Mifos Initiative > Release Manager » Apache Fineract > Author » Steadfast Self-Hosting > PGP key » 0xA9A14F22F57DA182 > >
