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

Improve issue management (closes #346). #347

Merged
merged 16 commits into from
Jun 2, 2020

Conversation

McCodeman
Copy link
Contributor

  • Added a new docs/labels.md to reference all labels used in
    GitHub issues and pull requests.
  • Added a visual developer workflow diagram at
    docs/assets/developer-workflow-opaque-bg.png (original src
    graffle added as well).
  • Updated CONTRIBUTING.md to reflect the improved issue
    management process.

@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-all: to trigger all tests.
  • /skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

@McCodeman
Copy link
Contributor Author

Some of the file changes in CONTRIBUTING.md due to reformatting markdown to 80 columns.

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

Adding Abhishek who wrote most of the original contributing.md guide

I think you're the only one who will be able to edit the graffle file :)

This does not document our commands for running tests, (/test-all, etc.) but these are likely to change in the near future and postponing to a future PR is likely the right thing to do.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/labels.md Outdated Show resolved Hide resolved
docs/labels.md Outdated Show resolved Hide resolved
docs/labels.md Outdated Show resolved Hide resolved
docs/labels.md Outdated Show resolved Hide resolved
docs/labels.md Outdated Show resolved Hide resolved
docs/labels.md Outdated Show resolved Hide resolved
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

found 2 more typos as I was giving this a second read through

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/github-labels.md Show resolved Hide resolved
docs/github-labels.md Show resolved Hide resolved
docs/github-labels.md Outdated Show resolved Hide resolved
docs/github-labels.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/github-labels.md Outdated Show resolved Hide resolved
docs/github-labels.md Outdated Show resolved Hide resolved
docs/github-labels.md Outdated Show resolved Hide resolved
docs/github-labels.md Outdated Show resolved Hide resolved
docs/issue-management.md Outdated Show resolved Hide resolved
docs/issue-management.md Outdated Show resolved Hide resolved
docs/issue-management.md Outdated Show resolved Hide resolved
docs/issue-management.md Outdated Show resolved Hide resolved
@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-all: to trigger all tests.
  • /skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

@McCodeman
Copy link
Contributor Author

/skip-all

CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/issue-management.md Outdated Show resolved Hide resolved
docs/issue-management.md Outdated Show resolved Hide resolved
@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-all: to trigger all tests.
  • /skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

2 similar comments
@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-all: to trigger all tests.
  • /skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-all: to trigger all tests.
  • /skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

Did another thorough review of this. LGTM, apart from a few things I pointed out. This has been opened for a while, so hopefully we can merge it soon.

| Label | Description | Added By |
|-------|-------------|----------|
| approved | Indicates a PR has been approved by OWNERS in accordance with [GOVERNANCE.md](GOVERNANCE.md) guidelines. | Maintainers |
| vmware-cla: no | Indicates the PR's author has not signed the (VMware CLA)[https://cla.vmware.com/faq] | VMware CLA Bot |
Copy link
Contributor

Choose a reason for hiding this comment

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

The link does not render properly: there can be no whitespace between (VMware CLA) and [https://cla.vmware.com/faq]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

Copy link
Contributor

Choose a reason for hiding this comment

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

still not fixed (when I look at the Github markdown rendering for the file), maybe they need to be on the same line...

|-------|-------------|----------|
| approved | Indicates a PR has been approved by OWNERS in accordance with [GOVERNANCE.md](GOVERNANCE.md) guidelines. | Maintainers |
| vmware-cla: no | Indicates the PR's author has not signed the (VMware CLA)[https://cla.vmware.com/faq] | VMware CLA Bot |
| vmware-cla: yes | Indicates the PR's author has signed the (VMware CLA)[https://cla.vmware.com/faq] | VMware CLA Bot |
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected.

Copy link
Contributor

Choose a reason for hiding this comment

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

same

docs/github-labels.md Outdated Show resolved Hide resolved
docs/issue-management.md Outdated Show resolved Hide resolved
docs/issue-management.md Show resolved Hide resolved
docs/issue-management.md Show resolved Hide resolved
docs/issue-management.md Outdated Show resolved Hide resolved
docs/issue-management.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/github-labels.md Outdated Show resolved Hide resolved
@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-windows-conformance: to trigger windows conformance tests.
  • /skip-windows-conformance: to skip windows conformance tests.
  • /test-all: to trigger all tests.
  • /skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

@McCodeman
Copy link
Contributor Author

/skip-all

@McCodeman
Copy link
Contributor Author

The forced push was due to a rebase to ensure we are reviewing latest set of docs.

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

Another thorough review and found a few more typos

The CI spell checker also found a couple

CONTRIBUTING.md Outdated
Once an issue has been submitted, the CI (GitHub actions) or a human will
automatically review the submitted issue or PR to ensure that it has all relevant
information. If information is lacking or there is another problem with the
submitted issue, an appropriate [`triage/<?>`](#triage) label will be
Copy link
Contributor

Choose a reason for hiding this comment

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

not a valid link, there is no triage section in this doc

CONTRIBUTING.md Outdated
applied.

After an issue has been triaged, the maintainers can prioritize the issue with
an appropriate [`priority/<?>`](#priority) label.
Copy link
Contributor

Choose a reason for hiding this comment

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

not a valid link, there is no priority section in this doc

CONTRIBUTING.md Outdated
* [`kind/feature`](docs/issue-management.md#feature) -- for proposing a feature
* [`kind/support`](docs/issue-management.md#support) -- to request support. You may also get support by
using our [Slack](https://kubernetes.slack.com/archives/CR2J23M0X) channel for
interactive help. If you have not setup the appropriate accounts, please
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/setup/set up
setup is the noun, set up is the verb

| Label | Description | Added By |
|-------|-------------|----------|
| approved | Indicates a PR has been approved by OWNERS in accordance with [GOVERNANCE.md](GOVERNANCE.md) guidelines. | Maintainers |
| vmware-cla: no | Indicates the PR's author has not signed the (VMware CLA)[https://cla.vmware.com/faq] | VMware CLA Bot |
Copy link
Contributor

Choose a reason for hiding this comment

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

still not fixed (when I look at the Github markdown rendering for the file), maybe they need to be on the same line...

|-------|-------------|----------|
| approved | Indicates a PR has been approved by OWNERS in accordance with [GOVERNANCE.md](GOVERNANCE.md) guidelines. | Maintainers |
| vmware-cla: no | Indicates the PR's author has not signed the (VMware CLA)[https://cla.vmware.com/faq] | VMware CLA Bot |
| vmware-cla: yes | Indicates the PR's author has signed the (VMware CLA)[https://cla.vmware.com/faq] | VMware CLA Bot |
Copy link
Contributor

Choose a reason for hiding this comment

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

same

processes.
* `help wanted` -- issues that represent clearly laid out tasks that are
generally tractable for new contributors. The solution has already been
designed and requires no further discussion from the community. This labels
Copy link
Contributor

Choose a reason for hiding this comment

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

s/This labels/This label

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected.

3. Apply [`size/<size>`](#size) label to the submission. (TODO: we plan to
automate this with a GitHub action and apply size based on lines of code).
4. Ensure that the PR references an existing issue (exceptions to this should be
rare). If the PR is missing this or needs any additional information, notate
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought "notate" was used specifically for music. Maybe "note it in a comment"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct. Changed.


### Size

Size labels beting with `size/<size>` and estimate the relative complexity or work
Copy link
Contributor

Choose a reason for hiding this comment

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

s/beting/begin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected. Not sure why spell check plugin not picking this up. Will investigate.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's my understanding that the spell checker only catches the most common spelling mistakes and does not lookup each work in a dictionary, as I would image there would be too many false positives for technical documentation.


### Triage

As soon as new issue are submitted, they must be triaged until they are ready to
Copy link
Contributor

Choose a reason for hiding this comment

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

s/new issue are submitted, they/new issues are submitted, they

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corrected.

TODO: Additional CI automation (GitHub actions) will be used to automatically
apply and manage some of these lifecycle labels.

Lifecycle labels are defined in [`docs/github-labels.md`](docs/github-labels.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

link not working because of missing leading slash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should have relative link so this work correctly in site docs. Have corrected.

@McCodeman
Copy link
Contributor Author

/skip-all

antoninbas
antoninbas previously approved these changes Jun 2, 2020
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

@antoninbas
Copy link
Contributor

@McCodeman actually looks like you haven't fixed the errors reported by the spell checker yet?

@antoninbas antoninbas self-requested a review June 2, 2020 04:58
* Added a new `docs/labels.md` to reference all labels used in
GitHub issues and pull requests.
* Added a visual developer workflow diagram at
  `docs/assets/developer-workflow-opaque-bg.png` (original src
  graffle added as well).
* Updated `CONTRIBUTING.md` to reflect the improved issue
  management process.
* whitespace, punctuation, and spelling corrections
* wording on purpose of size labels
* removed /approve-api-change until action ready
* added and changed area github labels for OVS and testing
* instructions apply to both issues and PRs
* changed name of labels.md to github-labels.md
* moved issue-management detail topics in `CONTRIBUTING.md` to
  `docs/issue-management.md`
* grammar, spelling, formatting corrections
* requested  information for upgrades on `kind/api-change`
* updated links to `github-labels.md`
* spelling, grammar, formatting, typo corrections
* simplified the section on filing-an-issue by merging with a
previous section on new issues
* consistent reference to "maintainers" and "owners"
* fixed broken links
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

@antoninbas
Copy link
Contributor

/skip-all

@antoninbas
Copy link
Contributor

Ignoring failed Kind test because this is purely a documentation PR

@antoninbas antoninbas merged commit 572524a into antrea-io:master Jun 2, 2020
GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Sep 22, 2020
* Improve issue management (closes antrea-io#346).

* Added a new `docs/labels.md` to reference all labels used in
GitHub issues and pull requests.
* Added a visual developer workflow diagram at
  `docs/assets/developer-workflow-opaque-bg.png` (original src
  graffle added as well).
* Updated `CONTRIBUTING.md` to reflect the improved issue
  management process.

* Added newline to end of .gitignore

* Typo fixes and suggestions for CONTRIBUTING.md and github-labels.md

* whitespace, punctuation, and spelling corrections
* wording on purpose of size labels
* removed /approve-api-change until action ready
* added and changed area github labels for OVS and testing
* instructions apply to both issues and PRs
* changed name of labels.md to github-labels.md

* Moved issue management detail topics to `docs/issue-management.md`

* moved issue-management detail topics in `CONTRIBUTING.md` to
  `docs/issue-management.md`
* grammar, spelling, formatting corrections
* requested  information for upgrades on `kind/api-change`
* updated links to `github-labels.md`

* Clarify good first issue and help wanted labels.

* spelling, grammar, formatting, typo corrections

* Added `area/component` labels.

* Added github label `area/interface`

* Added label `area/build-release`

* Corrected developer workflow visualization URL

* corrected typos

* Removed spurious asterix in priority bullet list

* Added `area/monitoring/health-performance

* Added link for how to use good first issue and help wanted.

* Spelling/typo corrections to issue-management.

* Documentation edits based on feedback

* simplified the section on filing-an-issue by merging with a
previous section on new issues
* consistent reference to "maintainers" and "owners"
* fixed broken links

* Fixed additional spelling, grammar, links.

* Fixed spelling errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants