+1 On Wed, May 13, 2026 at 9:37 PM Attila Budai <[email protected]> wrote:
> +1 > > On Thu, May 14, 2026, 3:02 AM Aman Mittal <[email protected]> > wrote: > >> +1 >> >> On Thu, 14 May, 2026, 5:30 am James Dailey, <[email protected]> wrote: >> >>> +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 >>>> >>>>
