-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add power event constants, types, and functions #1658
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
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.
Please add some tests to exercise the methods.
Please add some javadocs on the function mappings.
I just noticed that some of the code from |
The |
Done. |
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. One last request, please add a line in CHANGES.md.
Thanks for your contribution! I squashed your commits and will merge once CI passes. |
Thank you for your guidance! |
@dbwiddis please don't use githubs squash and merge (I assume that was used here). The function destroys authorship information and is broken. The commit resulting from this as author email "1707552+eranl@users.noreply.github.com". That is some BS introduced by github. This is kind of ironic as the real email address is listed as Co-authored-by. I see no problem squashing for contributors, but then please use the git CLI client or other non-broken tools. The git CLI client for example retains the author information of the original commits and just sets you as committer, which is, at least from my perspective is more correct, than this. |
No, I checked out the branch and combined the commits with an interactive rebase, then force-pushed to the PR branch in 646b23a There was a single commit on the PR when I merged it. My failure was in not looking at the commit itself which probably included the github "email" (if @eranl has not configured GitHub to sign with a real name) and editing the author information there. Sorry about that. |
@dbwiddis no problem, thanks for the explanation. So there is a second problem by the broken UI of github. |
Yep. Have seen it a lot at work, where we use DCO and malformed committees bite a lot. It would be helpful to me if you added something in the site about what to do/check before merging. I’ve mostly avoided it because I can never remember other than “don’t squash and merge” |
Add some missing power event constants, types, and functions.