asyncapi / .github

Location of all reusable community health files
31 stars 66 forks source link

Add support for minimum amount of approvals before merge #190

Open fmvilas opened 2 years ago

fmvilas commented 2 years ago

Have a look at https://github.com/asyncapi/spec/pull/851#issuecomment-1283573288.

Amzani commented 1 year ago

I would like to work on this issue.

Amzani commented 1 year ago

@fmvilas @derberg You want to require a minimum of 3 approvals for all AsyncAPI repos or you want to only to do it in some repos like specs ?

cc @KhudaDad414

KhudaDad414 commented 1 year ago

@Amzani I think we only need it to be 3 for specs. Besides we get the value from an environment variable so each repo can have their own value :)

Amzani commented 1 year ago

Great @KhudaDad414 this environment exists already ? or it's something new we have to create ?

KhudaDad414 commented 1 year ago

I don't think it exists yet. we can have a default value of 1 if it doesn't exist in the repo for now.

fmvilas commented 1 year ago

Yeah, the environment variable would actually be a secret in the repo: https://docs.github.com/en/actions/security-guides/encrypted-secrets. That's the only way to store info associated with a repo that comes to my mind. If you come up with a more elegant way to solve it, feel free!

octonawish-akcodes commented 1 year ago

Can I try this.

derberg commented 1 year ago

Can I try this.

first we need to figure the solution.


the best if that info is stored in a file in a repo, then any change goes through review. there is this concept of https://github.com/asyncapi/.github/issues/137 that we need anyway, not just amount of reviewers but for transparency reasons and increasing size of maintainers best if any repo setting is done through a PR

Amzani commented 1 year ago

I like the long term solution to solve this, then we could have 2 options

  1. A central configuration repo (e.g admin) in the AsyncAPI Github organisation that manages the settings for all repos. To implement it we could try something like Safe-settings
  2. Each repo defines its own settings. Something like

If we check the requirements listed in #137, the solutions I've mentioned can:

fmvilas commented 1 year ago

I honestly prefer a config file in each repo instead of a central one. This way code owners will have the power to review it and approve it.

derberg commented 1 year ago

yes, we need to have it per repo definitely (like https://github.com/repository-settings/app I guess), so only codeowners of given repo are involved in review, and they can adjust settings transparently however they need for they repository. Of course if there is a case that there are multiple repos that have exactly the same configuration, we can have it in .github repo and push out to repos that need it using the same automation we have for workflows or code of conduct. I'm referring to https://github.com/asyncapi/.github/blob/master/.github/workflows/global-replicator.yml

On requirements side, user management would be lovely too 😄 Basically anything that requires going to Settings so we can eliminate a need to access Settings in general.

Custom configuration (sonarcloud: true or coveralls: true) will be only possible in Safe-settings. the repository-settings doesn't support it, However it can be done by developing our custom app on top of probot and octokit-plugin-config

For others, like sonar or coveralls, yeah, we definitely will need something custom.

Most important is that we need a strong requirement here to make sure it all works like all else on GitHub Actions. So there are no apps, no deployments and related overhead. Just workflows that support sync of settings whenever they are modified. Maybe solution is to get important code from repository-setting and use in our custom action. Or maybe there is some action out there that we could already use or contribute to.

Afaik with existing mutations in GitHub GraphQL it is not that complicated after all. Updating all stuff with one shot is with one call. So the only complicated part I think is transform of YAML settings to API call.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

aakankshabhende commented 1 month ago

@derberg @Amzani I'm interested in working on this. Could you please assign me this?

aakankshabhende commented 1 month ago

Also, should the minimum approvals be 3?

derberg commented 4 weeks ago

it is definitely not developed yet

default should be 1 as it is now because of how things work, and per repo can be overwritten.

make sure to first explain how you want to solve it, before you open a PR and hit our review 😃

aakankshabhende commented 4 weeks ago

@derberg I believe that we can have a central config file in this repo for minimum approval, then for individual repo we can include a check if the that repository has its own override config (workflow), if not we will enforce the central workflow. I would love to hear any thoughts or ideas on how we can refine this approach.

derberg commented 4 weeks ago

@aakankshabhende please take into account that in AsyncAPI we use https://github.com/derberg/manage-files-in-multiple-repositories action that allows us to push workflows from .github to all the other repositories under asyncapi org. So we can have everywhere the same workflow that will be responsible for this approvals verification. No need for any central workflow.

As if it comes to configuration, instead of the config file, just like in some other issue Fran wrote, we can just have a system variable, that GitHub allows to have. So we have https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables on organization level, and workflow would read this variable, and if someone in some repository wants to overwrite this global default, they just overwrite the variable with custom value only in their repository

aakankshabhende commented 4 weeks ago

Got it. Sounds good! Thanks @derberg

aakankshabhende commented 4 weeks ago

I'll start working on this

derberg commented 4 weeks ago

awesome

please feel free to ask for any clarifications on the way - always better to clarify first than get ping-pong in a PR

if I do not answer here for some reason, feel free to DM me in AsyncAPI Slack

good luck!