Skip to content
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

OWNERS: Configure Prow with approver and reviewer information #71

Merged
merged 1 commit into from
Jul 26, 2018

Conversation

wking
Copy link
Member

@wking wking commented Jul 25, 2018

The folks I've listed here are all under @aaronlevel in Red Hat, with @yifan-gu being the installer lead. I'm guessing about roles, but we can always adjust later as we see fit.

Approvers can also /lgtm, so there's no need to list them under reviewers as well.

The docs link in OWNERS is from kubernetes/kubernetes-template-project#15.

While updating CONTRIBUTING.md to mention OWNERS (using wording based on this), I've also:

  • Added a line to make it clear that filing issues is helpful to (for folks who don't have the time or inclination to work up a PR).
  • Turned the unordered list into an ordered list. These entries happen sequentially, and an ordered list makes that more obvious.
  • Made "below" a link, to make finding the referenced content more convenient.
  • Shuffled words in the commit message format title to make it shorter.

The references to 'make structure-check' are stale since d61abd4 (coreos/tectonic-installer#3137), but I've left fixing that to follow-up work.

Fixes #32. CC @paulfantom.

@openshift-ci-robot openshift-ci-robot added retest-not-required-docs-only size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 25, 2018
@wking
Copy link
Member Author

wking commented Jul 25, 2018

The references to 'make structure-check' are stale since d61abd4 (coreos/tectonic-installer#3137), but I've left fixing that to follow-up work.

I have some local WIP for this, but I'll wait until this PR and #72 land (to avoid conflicts) before filing a PR.

@paulfantom
Copy link
Contributor

paulfantom commented Jul 25, 2018

Why @openshift-ci-robot reacted with emoji?! How?! I need to know! 😄

LGTM :shipit:

@wking
Copy link
Member Author

wking commented Jul 25, 2018

Why @openshift-ci-robot reacted with emoji?!

Prow's heart plugin likes new OWNERS files ;).

@paulfantom
Copy link
Contributor

Nice! 👍

@wking
Copy link
Member Author

wking commented Jul 25, 2018

/ok-to-test

@openshift-ci-robot openshift-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 25, 2018
Most of the folks I've listed here are under Aaron in Red Hat, with
Yifan being the installer lead.  I've also included Alex and Clayton,
who are higher up in Red Hat but still members of
@openshift/installer.  I'm guessing about roles, but we can always
adjust later as we see fit.

Approvers can also /lgtm [1], so there's no need to list them under
'reviewers' as well.

The docs link in OWNERS is from [2].

While updating CONTRIBUTING.md to mention OWNERS (using wording based
on [3]), I've also:

* Added a line to make it clear that filing issues is helpful to (for
  folks who don't have the time or inclination to work up a PR).
* Turned the unordered list into an ordered list.  These entries
  happen sequentially, and an ordered list makes that more obvious.
* Made "below" a link, to make finding the referenced content more
  convenient.
* Shuffled words in the commit message format title to make it
  shorter.

The references to 'make structure-check' are stale since d61abd4 (*:
cleanup bazel rules, 2018-03-26, coreos/tectonic-installer#3137), but
I've left fixing that to follow-up work.

[1]: https://github.com/kubernetes/community/blob/4c0c2e9e659d2f989f3d23c3634c16bb099f3064/contributors/guide/owners.md#quirks-of-the-process
[2]: kubernetes/kubernetes-template-project#15
[3]: https://github.com/kubernetes/kubernetes-template-project/blame/16f1588e6d4746876beb08cc64769657530d4872/CONTRIBUTING.md#L10
@wking
Copy link
Member Author

wking commented Jul 25, 2018

I've added @crawford and @smarterclayton as approvers, since they're also part of @openshift/installer.

@smarterclayton
Copy link
Contributor

/approve

I’ll stick the label on if everyone is gtg on this.

@smarterclayton
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 26, 2018
@openshift-merge-robot openshift-merge-robot merged commit 72b3ff6 into openshift:master Jul 26, 2018
@wking wking deleted the owners branch July 26, 2018 01:07
wking added a commit to wking/openshift-release that referenced this pull request Jul 30, 2018
The installer repo grew an OWNERS file with
openshift/installer@49779c3e (OWNERS: Configure Prow with approver and
reviewer information, 2018-07-25, openshift/installer#71), so now it
has approvers who are authorized to add the 'approved' label.  This
commit adjusts Tide to require that 'approved' label for installer
merges.

There are three Tide config groups with the same requirements:

* The one I'm moving openshift/installer to with this commit.

* One for openshift/origin.  The origin repo was moved to its own
  section in 070b90a (Exclude merging to openshift/origin#master,
  2018-04-09, openshift#761) to pick up an excludedBranches section.  But that
  excludedBranches section was dropped in 2bd76e3 (reenable merging
  to origin:master, 2018-06-28, openshift#1021).  However, we still want to
  keep origin in a separate section to make merge gating on rebases
  easier [1].  From Michalis [2]:

    When @openshift/sig-master want to land a kubernetes rebase in
    openshift/origin#master, we need to block merges on that branch
    and at the same time we don't want to be blocking merges
    elsewhere, hence origin has its own query.

  The installer repository doesn't need Kubernetes rebase gating, so
  it shouldn't go into this section.

* One for openshift/online-registration and
  openshift/enterprise-images.  This section was created in 5b004d2
  (Single out hidden repo in tide queries, 2018-03-22, openshift#707).  But the
  installer repo is public, so it shouldn't go into the hidden-repo
  section.

[1]: openshift#1122 (comment)
[2]: openshift#1122 (comment)
wking added a commit to wking/openshift-installer that referenced this pull request Aug 1, 2018
MAINTAINERS is obsolete since 49779c3 (OWNERS: Configure Prow with
approver and reviewer information, 2018-07-25, openshift#71).  I've also
dropped the email/IRC section, since as far as I know there are
currently no public lists or IRC channels for installer discussion.
Folks outside of Red Hat should communicate via GitHub issues and pull
requests, and I think that's the expectation for projects that don't
give alternatives in their CONTRIBUTING file.
@wking wking mentioned this pull request Aug 1, 2018
wking added a commit to wking/openshift-installer that referenced this pull request Aug 2, 2018
I can't spell.  Typo is from 49779c3 (OWNERS: Configure Prow with
approver and reviewer information, 2018-07-25, openshift#71).
frobware pushed a commit to frobware/installer that referenced this pull request Sep 17, 2018
MAINTAINERS is obsolete since 49779c3 (OWNERS: Configure Prow with
approver and reviewer information, 2018-07-25, openshift#71).  I've also
dropped the email/IRC section, since as far as I know there are
currently no public lists or IRC channels for installer discussion.
Folks outside of Red Hat should communicate via GitHub issues and pull
requests, and I think that's the expectation for projects that don't
give alternatives in their CONTRIBUTING file.
frobware pushed a commit to frobware/installer that referenced this pull request Sep 17, 2018
I can't spell.  Typo is from 49779c3 (OWNERS: Configure Prow with
approver and reviewer information, 2018-07-25, openshift#71).
stbenjam pushed a commit to stbenjam/installer that referenced this pull request Feb 10, 2021
Bug 1906935: Delete resources when Provisioning CR is deleted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. retest-not-required-docs-only size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add OWNERS file
6 participants