-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Update comparisons in test files from reflect to cmp #1875
Conversation
Can someone help me understand, why checks are failing? |
Looks like there is still a reference to the reflect package, but it's not imported any longer:
https://github.com/google/go-github/pull/1875/checks?check_run_id=2677192197#step:8:9 |
Thanks for replying, but I checked there is no "reflect" imported for that particular file in the branch I created PR with. |
It looks like you need to rebase onto the upstream master branch, as you're missing some commits that apparently are getting picked up by the CI jobs (notably d39b94d). I don't entirely understand why it's not simply running the CI jobs against your branch, which should pass, but would then have problems come merge time. Maybe it's automatically trying to merge your changes with master, resulting in these errors? |
Thanks @willnorris , I didn't realize my branch was not up to date. |
okay, so now the error about the reflect reference in |
Replaced all reflect.DeepEqual comparisons in test files with cmp.Equal. Fixes: google#1851
Codecov Report
@@ Coverage Diff @@
## master #1875 +/- ##
=======================================
Coverage 97.65% 97.65%
=======================================
Files 105 105
Lines 6783 6783
=======================================
Hits 6624 6624
Misses 86 86
Partials 73 73 Continue to review full report at Codecov.
|
Thanks for helping @willnorris . I have fixed the issue and seems like checks have also passed. |
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.
Thank you, @sagar23sj and @willnorris !
LGTM.
Awaiting second LGTM before merging.
@gmlewis oops, looks like I missed catching your issue rename before merging :) |
😂 No problem, Will! |
Replaced all reflect.DeepEqual comparisons in test files with cmp.Equal.
Fixes: #1851