Skip to content

Add CODEOWNERS file #7998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 3, 2022
Merged

Add CODEOWNERS file #7998

merged 1 commit into from
Feb 3, 2022

Conversation

mads-hartmann
Copy link
Contributor

@mads-hartmann mads-hartmann commented Feb 3, 2022

Description

As part of moving to using Github for PR review automation we are going to remove OWNERS in favour of CODEOWNERS (see RFC for details).

This PR adds the CODEOWNERS file. We will remove the OWNERS files once we have switched Tide to use Github Reviews (instead of the approved label, see gitpod-io/gitbot#65).

This can be merged now. The CODEOWNERS will then show up as "suggested" reviewers when you open a PR but reviews from CODEOWNERS are not required (we'll enable that later).

The CODEOWNERS file was generated using the following script and then modified slightly

Updated: there was a bug in the original script, see here. The script below has been updated in case we need to run it again)

#!/usr/bin/env bash

set -euo pipefail

function format-owner {
    for owner in $*
    do
        case $owner in
            engineering-platform)
                echo -n "@gitpod-io/platform "
                ;;
            engineering-*)
                echo -n "@gitpod-io/$owner "
                ;;
            *)
                echo -n "@$owner "
                ;;
        esac
    done
}

CODEOWNERS_FILE=/workspace/gitpod/.github/CODEOWNERS

rm -f "${CODEOWNERS_FILE}"
touch "${CODEOWNERS_FILE}"

shopt -s globstar nullglob
for OWNERS in **/OWNERS .werft/**/OWNERS; do
    owners="$(yq read --tojson "${OWNERS}" 'approvers' | jq -r 'join(" ")')"
    github_owners="$(format-owner $owners)"
    path="${OWNERS/\/OWNERS/}"
    echo "/$path $github_owners" >> "${CODEOWNERS_FILE}"
done

# Sort file based on path
sort -o "${CODEOWNERS_FILE}" "${CODEOWNERS_FILE}"

# Remoe trailing newlines (introduce by format-owner)
sed -i 's/\s+$//g' "${CODEOWNERS_FILE}"

Related Issue(s)

Part of https://github.com/gitpod-io/ops/issues/839

How to test

CODEOWNERS is hard to test. Once merged we should see the CODEOWNERS show up as suggested Reviewers when opening PRs.

Release Notes

NONE

Documentation

N/A

@codecov
Copy link

codecov bot commented Feb 3, 2022

Codecov Report

Merging #7998 (c99b31b) into main (95a36d8) will decrease coverage by 1.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7998      +/-   ##
==========================================
- Coverage   12.01%   10.86%   -1.16%     
==========================================
  Files          20       18       -2     
  Lines        1190     1022     -168     
==========================================
- Hits          143      111      -32     
+ Misses       1043      909     -134     
+ Partials        4        2       -2     
Flag Coverage Δ
components-gitpod-cli-app 10.86% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95a36d8...c99b31b. Read the comment docs.

@mads-hartmann mads-hartmann marked this pull request as ready for review February 3, 2022 09:32
@mads-hartmann mads-hartmann requested review from a team February 3, 2022 09:33
@github-actions github-actions bot added team: devx team: workspace Issue belongs to the Workspace team team: IDE team: webapp Issue belongs to the WebApp team team: delivery Issue belongs to the self-hosted team labels Feb 3, 2022
@csweichel
Copy link
Contributor

looks great. We should ask the team leads to double check that the GitHub teams are correct.

/lgtm

@roboquat
Copy link
Contributor

roboquat commented Feb 3, 2022

LGTM label has been added.

Git tree hash: e4fdebf5d0c63aea2c83e57278f0a6cdd8479479

@roboquat
Copy link
Contributor

roboquat commented Feb 3, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csweichel

Associated issue: #839

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 5f9825f into main Feb 3, 2022
@roboquat roboquat deleted the mads/add-codeowners branch February 3, 2022 09:35
@mads-hartmann
Copy link
Contributor Author

mads-hartmann commented Feb 3, 2022

@csweichel I was too slow to add the hold label so now this is merged 😬 I won't revert as the CODEOWNERS file doesn't do much yet. Before enabling branch protection rules to require reviews from code owners I'll ping the leads to verify the CODEOWNERS file looks correct.

/installer/pkg/components/ws-manager-bridge @gitpod-io/engineering-webapp
/installer/pkg/components/ws-manager @gitpod-io/engineering-workspace
/installer/pkg/components/ws-proxy @gitpod-io/engineering-workspace
/installer/pkg/config/versions @gitpod-io/engineering-ide @gitpod-io/engineering-ide @gitpod-io/engineering-ide @gitpod-io/engineering-ide
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think once is enough

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha this turned out to be a bug. Fixing in upcoming PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was fixed in https://github.com/gitpod-io/gitpod/pull/8000/files and there were not other cases of multiple teams being approvers in the same OWNERS file

# A single review is enough. Usually the review will be done by RoboQuat but we don't want to add
# that user as an owner in case we need to manually approve changes
#
/CHANGELOG.md
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. That should make the changelog job a lot easier!

@kylos101
Copy link
Contributor

kylos101 commented Feb 3, 2022

For posterity I reviewed the CODEOWNERS file and LGTM.

@roboquat roboquat added the deployed: workspace Workspace team change is running in production label Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved deployed: workspace Workspace team change is running in production release-note-none size/M team: delivery Issue belongs to the self-hosted team team: devx team: IDE team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants