opensearch-project / .github

Provides templates and resources for other OpenSearch project repositories.
Apache License 2.0
29 stars 71 forks source link

[PROPOSAL] Build a GitHub app / workflow that runs on each PR that modifies the CODEOWNERS and MAINTAINERS files to make sure they are in sync #177

Open AMoo-Miki opened 1 year ago

AMoo-Miki commented 1 year ago

What/Why

What are you proposing?

A method for preventing CODEOWNERS and MAINTAINERS from drifting apart.

What users have asked for this feature?

Repository maintainers would like to have a workflow / app that examines changes made to CODEOWNERS and MAINTAINERS in a PR and fail if they are not in sync.

What problems are you trying to solve? Why should it be built? Any reason not to?

Repository maintainers are not a fan of manually reviewing changes made to CODEOWNERS and MAINTAINERS to make sure they are in sync.

What is the developer experience going to be?

A developer making changes to either of CODEOWNERS and MAINTAINERS files will have their PRs fail if the two files are not in sync. Developers of PRs that have no changes to CODEOWNERS and MAINTAINERS will have the workflow skipped.

Are there any security considerations?

The workflow / app will not have access to verify the accuracy of the CODEOWNERS and MAINTAINERS files; it only checks if they are in sync.

hakunamatata-git commented 11 months ago

Hello @AMoo-Miki @peternied , I am new to contributing to opensource project. I would like to work on this. I have checked the MAINTAINERS.md and CODEOWNERS files. They both not following the similar format. Eg: MAINTAINERS.md this file is displaying the "Maintainer | GitHub ID | Affiliation" where as CODEOWNERS file displaying "@XYZ @ABC @123 @abc @xyz"

If I do diff, or delta or compare the comparison fails. The contents of the file is not same. May I know exactly what needs to be checked before failing the PR. Does CODEOWNERS file must look same as MAINTAINERS file or vice versa?

Thanks in advance.

peternied commented 11 months ago

@hakunamatata-git Thanks for your interest, the files are not in the same format so diffs would be expected not to match. Coming up with a structured way to read the files and interpret the different maintainers definitions would be part of this work to resolve this issue.

dblock commented 11 months ago

I have working code that reads/extracts MAINTAINERS and CODEOWNERS here: https://github.com/opensearch-project/project-tools/blob/69f056b6ab1c2868c54537e2a111081da569fc59/lib/github/repo.rb#L39, https://github.com/opensearch-project/project-tools/blob/69f056b6ab1c2868c54537e2a111081da569fc59/lib/github/repo.rb#L101.

hakunamatata-git commented 10 months ago

Hello @peternied @AMoo-Miki @dblock I have wrote this yaml. Can some one please validate if I am on the right path. sync-check.yaml.zip

dblock commented 10 months ago
      - name: Check if .github/CODEOWNERS and MAINTAINERS.md are in sync
        run: |
          if [ "$(cat .github/CODEOWNERS)" != "$(cat MAINTAINERS.md)" ]; then
            echo ".github/CODEOWNERS and MAINTAINERS.md files are not in sync."
            exit 1
          fi

The contents of these files are completely different, they need to be parsed.

hakunamatata-git commented 10 months ago

Hello @dblock @peternied @AMoo-Miki Thank you for the response. I have parsed the files and ran locally to test the script. Please validate and let me know if I'm in the right direction. compare_files.zip

dblock commented 10 months ago

Please validate and let me know if I'm in the right direction.

Thanks!

Comparison is only one part of the problem. Your implementation is not as generic as the Ruby code I linked, so I think it's not the right direction, plus going from Ruby to Python is not the goal.

We're looking for a GitHub action that one can reuse in any repo that performs the task asked here. Examples of reusable GitHub actions are on the GitHub marketplace: https://github.com/marketplace. I want to be able to write something like this:

- name: Check CODEOWNERS vs. MAINTAINERS
   uses: actions/codeowners-maintainers@v1

And it should be appending errors to the GitHub pull request when these are out of sync.

Btw, you should be creating GitHub repos for your code, don't attach zips ;)