Skip to content

Let users specify unique function keys using delay.MustRegister #268

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

Merged
merged 3 commits into from
Nov 5, 2021

Conversation

KaylaNguyen
Copy link
Collaborator

No description provided.

@google-cla google-cla bot added the cla: yes label Nov 2, 2021
@KaylaNguyen KaylaNguyen requested a review from zevdg November 2, 2021 00:56
@zevdg
Copy link
Collaborator

zevdg commented Nov 2, 2021

I'm a little confused. At a glance, this looks like the same exact change as #261 just on the master branch instead of qa and squashed to 1 commit. Is that right? If so, this SGTM. We made this change in a totally backwards compatible way so backporting it to master makes total sense.

Since we already reviewed the code in #261, it seems like the approver for this CL should be verifing that you've backported the fix from qa to master correctly. If your team has a process for this, I don't know what it is, so I can't verify that you've followed it correctly. If they don't have a process and you're flying by the seat of your pants, that's ok for now, but you should document how you did this one as a baseline and I can at least review that process.

@KaylaNguyen
Copy link
Collaborator Author

Yes, this commit is an exact replica of #261 but to master branch. We don't have a process for this so I'm just porting this change manually.

@zevdg
Copy link
Collaborator

zevdg commented Nov 5, 2021

Did you run some git commands to create this commit? I'd feel much better about approving this if it was created by a git merge --squash. If not, could you try recreating this change using git merge --squash?

@KaylaNguyen
Copy link
Collaborator Author

TIL, thanks!
I did
→ git checkout master
→ git merge --squash delaypkg
→ git commit ....
→ git checkout delaypkg-master
→ git merge qa-upstream

A bit roundabout but I think that works.

@KaylaNguyen KaylaNguyen merged commit ab4e2d3 into golang:master Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants