Skip to content
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

Fix #1083 (Nil-pointer dereference) #1084

Merged
merged 5 commits into from
Aug 24, 2021
Merged

Conversation

mvrahden
Copy link
Contributor

The fix gracefully fails the assertion when given nil as list value instead of panicking unresolved.

Fixes #1083

the fix gracefully fails the assertion instead of panicking unresolved.
@mvrahden mvrahden changed the title Fix #1083 Fix #1083 (Nil-pointer dereference) May 30, 2021
@mvrahden
Copy link
Contributor Author

@ernesto-jimenez can anybody give a pass on this please? this is a trivial fix IMHO

@mvrahden
Copy link
Contributor Author

mvrahden commented Jul 3, 2021

As recommended in #1095 I'll kindly ask @boyan-soubachov to review this fix and provide feedback or merge.

Copy link
Collaborator

@boyan-soubachov boyan-soubachov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I think we need to migrate to GitHub actions since Travis killed support for open source a while ago if I recall correctly :/

@@ -721,7 +721,11 @@ func NotEqualValues(t TestingT, expected, actual interface{}, msgAndArgs ...inte
func includeElement(list interface{}, element interface{}) (ok, found bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something seems wrong with the docstring of this function... Do you mind updating the function name in line 717?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, they are not synced. I fixed that and fixed further linting issues in the assert package.

@boyan-soubachov
Copy link
Collaborator

I've been a bit swamped with life for the past year or so (who knew 2020 and 2021 would be such shockers :D)

Thanks for your contribution. I'll first migrate the builds to GitHub actions and we can then rebase and merge your PR once the builds are done :)

@mvrahden
Copy link
Contributor Author

mvrahden commented Jul 4, 2021

All good :) the last year was a bummer for many I guess 😅

I addressed your request with two additional commits.

There are further linting issues in other packages.
I might get back to them when I have some time and will make another PR for them.

@boyan-soubachov
Copy link
Collaborator

Hello, do you mind merging the latest master or rebasing this PR to trigger the CI builds

@mvrahden
Copy link
Contributor Author

mvrahden commented Jul 8, 2021

should be rebased now 👍

@boyan-soubachov
Copy link
Collaborator

LGTM, thank you for your contribution!

@mvrahden
Copy link
Contributor Author

mvrahden commented Jul 9, 2021

happy to do so!

@boyan-soubachov boyan-soubachov merged commit ab6dc32 into stretchr:master Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent nil pointer dereference
2 participants