First stab at MAINTAINERS.md & OWNERS.md & CONTRIBUTING.md by oschaaf · Pull Request #238 · envoyproxy/nighthawk

oschaaf · 2019-12-15T21:28:08Z

A pre-merge checklist
Documents what goes into an update to the Envoy dependency

Fixes #230

Signed-off-by: Otto van der Schaaf oschaaf@we-amp.com

- A pre-merge checklist - Documents what goes into an update to the Envoy dependency Fixes envoyproxy#230 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

caniszczyk · 2019-12-16T01:20:02Z

Where is the actual list of maintainers for the project?

oschaaf · 2019-12-16T08:50:01Z

Possibly I am using the wrong terminology here; this document targets to be helpful to reviewers.

htuch

Thanks for putting this together, CC @envoyproxy/nighthawk-maintainers

htuch · 2019-12-16T19:18:14Z

MAINTAINERS.md

+# Maintainers
+
+This document aims to assist maintainers.
+


Can you add some stuff on how to spot which changes are Nighthawk local vs. those that require deeper Envoy internals and architecture knowledge?

htuch · 2019-12-16T19:29:28Z

@caniszczyk good point. Can we have an OWNERS.md file @oschaaf in the same style as https://github.com/envoyproxy/envoy/blob/master/OWNERS.md?

We should also link to the @envoyproxy/nighthawk-maintainers team page.

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf · 2019-12-16T22:31:46Z

@envoyproxy/nighthawk-maintainers please check the newly created OWNERS.md in this PR; it has email addresses and suggests areas of expertise indicating what should be routed to whom; both need feedback.

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

htuch

Thanks, looks great. We're finally a real project with governance :) !

CONTRIBUTING.md

OWNERS.md

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf · 2019-12-17T09:36:21Z

Thanks for the feedback @htuch, based on that I pushed 009ab22

mum4k · 2019-12-17T14:10:26Z

CONTRIBUTING.md

+
+# Communication
+
+* Before starting work on a major feature, please reach out to us via GitHub, Slack,


Should we include link to or at least mention the Slack workspace we are using?

CONTRIBUTING.md

mum4k · 2019-12-17T14:13:46Z

CONTRIBUTING.md

+
+* Generally Nighthawk mirrors [Envoy's policy](https://github.com/envoyproxy/envoy/blob/master/CONTRIBUTING.md#pr-review-policy-for-maintainers) with respect to PR submission policy.
+* Any PR that changes user-facing behavior **must** have associated documentation in [docs](docs) as
+  well as [release notes](docs/root/intro/version_history.rst). 


This file doesn't seem to exist, while a version_history.md directly under root does:

https://github.com/envoyproxy/nighthawk/blob/master/docs/root/version_history.md

CONTRIBUTING.md

MAINTAINERS.md

OWNERS.md

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf

@mum4k comments addressed in 66ba23d

CONTRIBUTING.md

htuch · 2019-12-19T04:51:55Z

CONTRIBUTING.md

+
+# PR review policy for maintainers
+
+* Generally Nighthawk mirros [Envoy's policy](https://github.com/envoyproxy/envoy/blob/master/CONTRIBUTING.md#pr-review-policy-for-maintainers) with respect to maintainer review policy.


Nit: mirrors

htuch · 2019-12-19T04:52:19Z

CONTRIBUTING.md

+
+# DCO: Sign your work
+
+Commits need to be signed off. [See here](https://github.com/envoyproxy/envoy/blob/master/CONTRIBUTING.md#dco-sign-your-work).


Nit: I would put the See outside of the link

htuch · 2019-12-19T04:53:15Z

MAINTAINERS.md

+
+As a guideline, concepts in Nighthawk that are derived from Envoy
+require someone with Envoy domain expertise in review. Notable examples
+are the way Nighthawk internal computes cluster configuration, its


Nit: "Nighthawk internally"

htuch · 2019-12-19T04:53:53Z

MAINTAINERS.md

+
+- Does the PR have breaking changes? Then that should be explicitly mentioned in the [version history](docs/root/version_history.md).
+- New features should be added to the [version history](docs/root/version_history.md).
+- Breaking changes to the [proto apis](api/) are not allowed.


Nit: "protobuf APIs"

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf · 2019-12-19T07:50:13Z

@htuch nits fixed in 7ff2dd2

htuch

Thanks!

mum4k

Thank you Otto, looks good. One remaining comment.

CONTRIBUTING.md

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

…y#238) * First stab at MAINTAINERS.md - A pre-merge checklist - Documents what goes into an update to the Envoy dependency Fixes envoyproxy#230 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com> * Review feedback: CONTRIBUTING.md & OWNERS.md Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com> * Review-feedback: Envoy domain expertise section Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com> * Review feedback Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com> * Review feedback Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com> * Review: fix nits Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com> * Review: fix bad link, .rst -> .md Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com> Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com>

First stab at MAINTAINERS.md

8adda5d

- A pre-merge checklist - Documents what goes into an update to the Envoy dependency Fixes envoyproxy#230 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

htuch reviewed Dec 16, 2019

View reviewed changes

oschaaf added 2 commits December 16, 2019 22:29

Merge remote-tracking branch 'upstream/master' into maintainers-doc

eadeb66

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Review feedback: CONTRIBUTING.md & OWNERS.md

47762c3

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Review-feedback: Envoy domain expertise section

214a037

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf added the needs-feedback label Dec 16, 2019

oschaaf changed the title ~~First stab at MAINTAINERS.md~~ First stab at MAINTAINERS.md & OWNERS.md & CONTRIBUTING.md Dec 16, 2019

htuch suggested changes Dec 17, 2019

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

OWNERS.md Show resolved Hide resolved

OWNERS.md Outdated Show resolved Hide resolved

Review feedback

009ab22

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf added waiting and removed needs-feedback labels Dec 17, 2019

mum4k self-requested a review December 17, 2019 14:20

mum4k requested changes Dec 17, 2019

View reviewed changes

oschaaf added P2 and removed waiting labels Dec 17, 2019

Review feedback

66ba23d

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf commented Dec 18, 2019

View reviewed changes

CONTRIBUTING.md Show resolved Hide resolved

htuch reviewed Dec 19, 2019

View reviewed changes

oschaaf added 2 commits December 19, 2019 08:43

Merge remote-tracking branch 'upstream/master' into maintainers-doc

f5e0521

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Review: fix nits

7ff2dd2

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

htuch previously approved these changes Dec 19, 2019

View reviewed changes

mum4k self-assigned this Dec 20, 2019

mum4k requested changes Dec 20, 2019

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

Review: fix bad link, .rst -> .md

63710cf

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf dismissed htuch’s stale review via 63710cf December 20, 2019 07:35

oschaaf mentioned this pull request Dec 20, 2019

Rename version_history.md to version_history.rst #244

Open

oschaaf added the waiting-for-review A PR waiting for a review. label Dec 20, 2019

mum4k approved these changes Dec 21, 2019

View reviewed changes

mum4k removed the waiting-for-review A PR waiting for a review. label Dec 21, 2019

mum4k merged commit 52199ba into envoyproxy:master Dec 21, 2019


		# Communication

		* Before starting work on a major feature, please reach out to us via GitHub, Slack,


		# PR review policy for maintainers

		* Generally Nighthawk mirros [Envoy's policy](https://github.com/envoyproxy/envoy/blob/master/CONTRIBUTING.md#pr-review-policy-for-maintainers) with respect to maintainer review policy.


		# DCO: Sign your work

		Commits need to be signed off. [See here](https://github.com/envoyproxy/envoy/blob/master/CONTRIBUTING.md#dco-sign-your-work).

Conversation

oschaaf commented Dec 15, 2019

Uh oh!

caniszczyk commented Dec 16, 2019

Uh oh!

oschaaf commented Dec 16, 2019

Uh oh!

htuch left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

htuch commented Dec 16, 2019

Uh oh!

oschaaf commented Dec 16, 2019

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oschaaf commented Dec 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oschaaf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oschaaf commented Dec 19, 2019

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

mum4k left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

htuch left a comment •

edited

Loading