+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
>>>
>>>

Reply via email to