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

Best practice for mailing list and github notification #3777

Merged
merged 1 commit into from Jul 1, 2019

Conversation

@nzoueidi
Copy link
Member

commented Jun 5, 2019

I would like to have your feedback and inputs for the Mailing list and Github notifications best practice.
I am thinking that the two parts need to be in the same page for more simplicity.

Once, there is agreement about this file, I will add the links to the top level README and then also to the communication section of the contributor guide.

Fixes #1724

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jun 5, 2019

Hi @nzoueidi. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@nzoueidi

This comment has been minimized.

Copy link
Member Author

commented Jun 5, 2019

Also, I think the second part of mailing list should have more steps (steps for creating labels and apply them, etc)..

/sig contributor-experience
/area contributor-guide

@cblecker

This comment has been minimized.

Copy link
Member

commented Jun 5, 2019

/ok-to-test
/assign @castrojo

Assigning Jorge for review.

One thing of note, I'm not sure the root of communication/ is the right place for this. Also, we typically go with lowercase filenames for these docs, rather than mixed case.

@nzoueidi

This comment has been minimized.

Copy link
Member Author

commented Jun 5, 2019

One thing of note, I'm not sure the root of communication/ is the right place for this. Also, we typically go with lowercase filenames for these docs, rather than mixed case.

I was going to create the file with lowercase, but it will be too long. It would be something like mailing-list-github-notification-best-practice.md or it wouldn't be obvious if I keep it the same name but with lowercase mailinglist-githubnotification-best-practice.md.

I think though about mailing-list-github-usage.md, what do you think?

@nzoueidi nzoueidi referenced this pull request Jun 6, 2019

Closed

REQUEST: New membership for nzoueidi #890

6 of 6 tasks complete
@castrojo

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2019

I would just go with communication/best-practices.md to keep it simple and then let the title speak for itself.

It will look right when I put it in the right order in the table of contents for the entire contributor guide with the page descriptions.

Thanks for working on this, let's do the rename and I'll approve and merge, thanks!

@nzoueidi nzoueidi force-pushed the nzoueidi:1724 branch from b82e14e to f2b6aa0 Jun 6, 2019

@geekygirldawn

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

This is a great addition! The one thing I'm wondering about is whether the instructions should be specific to Gmail or more general to apply to other email clients. With that said, enough of us use Gmail that I don't really object to having the instructions be specific :)

Show resolved Hide resolved communication/best-practices.md Outdated
Show resolved Hide resolved communication/best-practices.md Outdated
Show resolved Hide resolved communication/best-practices.md Outdated
Show resolved Hide resolved communication/best-practices.md Outdated
Show resolved Hide resolved communication/best-practices.md Outdated
Show resolved Hide resolved communication/best-practices.md Outdated
Show resolved Hide resolved communication/best-practices.md Outdated
Show resolved Hide resolved communication/best-practices.md Outdated
Show resolved Hide resolved communication/best-practices.md Outdated
Show resolved Hide resolved communication/best-practices.md Outdated

@nzoueidi nzoueidi force-pushed the nzoueidi:1724 branch from 62d8b1a to 3cd34d1 Jun 17, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jun 17, 2019

@castrojo

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jun 19, 2019

@nzoueidi

This comment has been minimized.

Copy link
Member Author

commented Jun 19, 2019

Last time, it failed due to the pod pending timeout. Triggering another build to make sure that everything is fine.

/test pull-community-verify

@parispittman

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

@nzoueidi @castrojo can we still include a link to gubernator in the meantime as an additional resource?

@nzoueidi nzoueidi force-pushed the nzoueidi:1724 branch from 3cd34d1 to d4a93c7 Jun 20, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jun 20, 2019

@nzoueidi

This comment has been minimized.

Copy link
Member Author

commented Jun 20, 2019

@nzoueidi @castrojo can we still include a link to gubernator in the meantime as an additional resource?

Thanks @parispittman just added it. PTAL.

Add Mailing list and Github notification best practice
  * Rename the file to best-practices.md
  * Addressing reviewers's comments
  * Reference Tim's gmail filters in kubernetes-dev
  * Address some of the comments
  * Fix some typos
  * Address reviews
  * Add Gubernator note
  * Use git.k8s.io instead of github.com

@nzoueidi nzoueidi force-pushed the nzoueidi:1724 branch from d4a93c7 to 8ccc1d1 Jun 20, 2019

@fejta
Copy link
Contributor

left a comment

/lgtm

@justaugustus

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

I think is looking pretty great. Can we merge and iterate over any remaining feedback?

@idvoretskyi
Copy link
Member

left a comment

I think is looking pretty great. Can we merge and iterate over any remaining feedback?

+1.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: idvoretskyi, nzoueidi

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

@k8s-ci-robot k8s-ci-robot merged commit ec8f985 into kubernetes:master Jul 1, 2019

4 checks passed

cla/linuxfoundation nzoueidi authorized
Details
pull-community-tempelis-check Skipped.
pull-community-verify Job succeeded.
Details
tide In merge pool.
Details
@nikhita

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

Retroactive lgtm. Thanks again for getting this in, @nzoueidi!! 💯 🥇

@justaugustus

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

Yayyyyyyyyyyyyyy 🚀

@nzoueidi

This comment has been minimized.

Copy link
Member Author

commented Jul 5, 2019

Back to this, I think we should communicate about this in k-dev. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.