Approved roles

[Keyrxng]

Resolves #18

• filters reviews based on the allowedRoles previously used for assigning the timeout requirements
• added test and confirmed with local script on the original context issue
• yarn format made a few changes
• added package_manager field to package.json

[gentlementlegen]

allowedReviewerRoles might be more explicit here.

[gentlementlegen]

I think there is a logic issue. Take the following scenario:
In the configuration, I set allowedReviewerRoles to ["OWNER"]. When the merge timeout will try to get the deadline requirements, a MEMBER would be considered as a COLLABORATOR instead of CONTRIBUTOR because COLLABORATOR is not in the allowedReviewerRoles.

Example run: https://github.com/Meniole/automated-merging/actions/runs/10748866022/job/29813121322#step:6:68

[Keyrxng]

I think there is a logic issue. Take the following scenario: In the configuration, I set allowedReviewerRoles to ["OWNER"]. When the merge timeout will try to get the deadline requirements, a MEMBER would be considered as a COLLABORATOR instead of CONTRIBUTOR because COLLABORATOR is not in the allowedReviewerRoles.

Example run: https://github.com/Meniole/automated-merging/actions/runs/10748866022/job/29813121322#step:6:68

I'm a little confused by what you are implying. I think you are suggesting that the deadline requirement array should be the same as it was before that way they'd be assigned COLLABORATOR and not the default CONTRIBUTOR in the event that the allowedReviewerRoles does not contain COLLABORATOR, is that right @gentlementlegen?

[gentlementlegen]

With the following configuration

  - uses:
    - plugin: ubiquity/automated-merging@development
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"
        repos:
          monitor: [ ]
          ignore: ["conversation-rewards"]
        allowedReviewerRoles: ["OWNER"]

Someone with the COLLABORATOR role would not be able to be considered to be counted within the reviewers, which is ok and intended. However if the collaborator is the one opening the pull-request, its timing for closing would be the ones for contributor instead of collaborator because the array considered to check the merge timeout is only OWNER. Hope it is clear enough.

[0x4007]

What is an "owner" role? There should only be, basically: admin, assignee, collaborator, contributor.

Is owner the pull request creator?

[gentlementlegen]

OWNER is the creator of the repository. I took this as an arbitrary example, same would apply if I used ADMIN, that would be considered as a CONTRIBUTOR for merging timeout in that example.

[0x4007]

Can you fix your yarn

[Keyrxng]

With the following configuration

  - uses:
    - plugin: ubiquity/automated-merging@development
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"
        repos:
          monitor: [ ]
          ignore: ["conversation-rewards"]
        allowedReviewerRoles: ["OWNER"]

Someone with the COLLABORATOR role would not be able to be considered to be counted within the reviewers, which is ok and intended. However if the collaborator is the one opening the pull-request, its timing for closing would be the ones for contributor instead of collaborator because the array considered to check the merge timeout is only OWNER. Hope it is clear enough.

Not sure how else to address the scenario that you raised apart from retaining your original implementation and hardcoding the timeout roles.

1. Potential user errors using the same allowedReviewerRoles item and additional unneeded complexity adding a new config item for this array separately.
2. I figured that specific aspect is author-defined like the way skipBotsEvents should be as oppose to author defined.

Can you fix your yarn
I can't find your comment rn but didn't we agree that we'd standardize this? No harm done including it but potential room for error without it. I stated it as an addition in the spec body too.

[0x4007]

Don't understand your message. What's the problem?

[gentlementlegen]

Seems that it works properly, my last QA: https://github.com/Meniole/automated-merging/actions/runs/10828231151/job/30043117944#step:6:68
Automated merge: Meniole#19

[Keyrxng]

Don't understand your message. What's the problem?

I don't understand your message now lmao.

• If you mean the issue raised by mentlegen, I was unsure myself as I expected allowedReviewerRoles to always be the same for fetching timeouts and fetching authorized PR approvals, they appear like they'd always rely on the same roles. I scratched my head with a solve and revert to original was the only guaranteed way to avoid the scenario raised.
• If you mean packageManager then the docs detail things better than I do but it was a prominent issue when we were using multiple yarn versions across plugins and packages etc. I'm sure we standardized to yarn@v1.22 org wide though so it makes sense to declare that I thought

This feature simplifies two core workflows:

It eases new contributor onboarding, since they won't have to follow system-specific installation processes anymore just to have the package manager you want them to.

It allows you to ensure that everyone in your team will use exactly the package manager version you intend them to, without them having to manually synchronize it each time you need to make an update.

[0x4007]

@gentlementlegen why did UbiquityOS handle the merge?

Would be funny if it read the conversation and picked up on the vibes to merge.

[gentlementlegen]

@0x4007 I noticed that the configuration was set to 5 minutes for collaborators, probably for testing, I corrected that afterwards so it doesn't happen again. Maybe we could do sentiment analysis and eventually merge lol

[0x4007]

+ ~ * checking vibes, please wait... * ~ +