-
Notifications
You must be signed in to change notification settings - Fork 5
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
Sc 14489: contact migration #1012
Conversation
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.
Makes sense to me! The plan is to just keep this in the branch until the migration is done right?
cmd/gdsutil/main.go
Outdated
} | ||
} else { | ||
modelContact.Vasps = append(modelContact.Vasps, vasp.CommonName) | ||
modelContact.EmailLog = append(modelContact.EmailLog, vaspContactExtra.EmailLog...) |
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.
Does this work if vaspContactExtra.EmailLog
is nil?
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.
whoops, I checked and it doesn't, fixed
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.
Thanks for checking!
if c.Bool("dryrun") { | ||
fmt.Println("existing vasp contacts:") | ||
for _, contact := range vaspContacts { | ||
fmt.Print(contact) |
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.
Might want to newline separate these or something or just print out the emails space separated because this is probably going to be messy when things get printed out.
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.
Good Idea!
for iter.Next() { | ||
vaspContact, _ := iter.Value() | ||
vaspContacts = append(vaspContacts, vaspContact) | ||
modelContact, AlreadyExists := modelContacts[vaspContact.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.
We may need to make sure the email is normalized before putting it in the map, although if it's coming from the VASP hopefully it will already be.
There is a story for Ben in the email verification epic to do the migration using this command. That will need to happen by Friday so we should probably just go ahead and merge this. |
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.
Scope of changes
Adds a new gdsutil command to migrate vasp contacts to the model contacts namespace.
Type of change
Acceptance criteria
Describe how reviewers can test this change to be sure that it works correctly. Add a checklist if possible
Author checklist
Reviewer(s) checklist