-
Notifications
You must be signed in to change notification settings - Fork 5.2k
validate emails .... #8377
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
validate emails .... #8377
Conversation
Ok good, this now fails on the currently mismatched and/or missing emails: https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/community/8377/pull-community-verify/1899592384602181632 |
…th de-normalized person entries
these leads had emails set but not the same across all positions, reconciled them (Note TODO about data format ...)
confirmed with them in slack
@@ -3995,7 +4008,7 @@ committees: | |||
- github: aojea | |||
name: Antonio Ojea | |||
company: Google | |||
email: antonio.ojea.garcia@gmail.com | |||
email: aojea@google.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aojea this was the one set the most for you, and the oldest one, but we can update it to either (just need it to be the same for all so we don't duplicate to CNCF)
The rest of these were only "inconsistent" for having empty values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with any
@@ -3382,6 +3387,7 @@ sigs: | |||
- github: marosset | |||
name: Mark Rossetti | |||
company: Microsoft | |||
email: [email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confirmed with @marosset in slack
aside from marosset and aojea, all others are just filling in empty fields to match existing entries for other groups
ideally we would dedupe the person entries entirely, but that's a larger PR, and they're usually duplicated at most a few times ...
/hold |
/lgtm |
/lgtm for me |
we might need to update Lines 3 to 6 in 1eefeb5
|
cc @kubernetes/sig-contributor-experience-leads |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, soltysh 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 |
/hold-cancel |
LGTM – changes for enforcing setting email Also, re: #8377 (comment) I'll discuss it among contribex leads and will follow up with a PR (most likely, will add contrbex leads alias to start with) |
/hold cancel |
// non-emeritus must have email and company set | ||
if prefix != "emeritus_lead" { | ||
// email must be set and consistent | ||
if val.Email == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And add a TODO about awkward data format with de-normalized person entries.
TODO: once I've confirmed that CI fails with the first commit, fix inconsistent entries in a second commit.
Refer to kubernetes/steering#281
Which issue(s) this PR fixes:
Fixes #