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

NOISSUE - Fix Contains function for nil arguments #1102

Merged
merged 1 commit into from
Apr 7, 2020

Conversation

blokovi
Copy link
Contributor

@blokovi blokovi commented Apr 6, 2020

Signed-off-by: Ivan Milošević [email protected]

What does this do?

Fix bug in Contains function in error package.
Bug was that Contains would return true when second argument is nil regardless on value of first argument

Which issue(s) does this PR fix/relate to?

No issue

@blokovi blokovi requested a review from a team as a code owner April 6, 2020 21:29
@codecov-io
Copy link

codecov-io commented Apr 6, 2020

Codecov Report

Merging #1102 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1102      +/-   ##
==========================================
- Coverage   77.13%   77.10%   -0.03%     
==========================================
  Files          95       95              
  Lines        6769     6769              
==========================================
- Hits         5221     5219       -2     
- Misses       1211     1213       +2     
  Partials      337      337              
Impacted Files Coverage Δ
things/service.go 88.97% <0.00%> (-1.48%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 382bc89...1b839b9. Read the comment docs.

manuio
manuio previously approved these changes Apr 6, 2020
@drasko drasko requested a review from dborovcanin April 6, 2020 23:29
@drasko
Copy link
Contributor

drasko commented Apr 6, 2020

@dusanb94 needs to approve this one

drasko
drasko previously approved these changes Apr 6, 2020
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

LGTM

@drasko drasko dismissed their stale review April 6, 2020 23:32

Removin my approval to prevent accidental merge before Dusan reviews.

dborovcanin
dborovcanin previously approved these changes Apr 7, 2020
@blokovi blokovi dismissed stale reviews from dborovcanin and manuio via 1b839b9 April 7, 2020 08:24
Copy link
Contributor

@manuio manuio left a comment

Choose a reason for hiding this comment

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

LGTM

@manuio manuio merged commit 338c55e into absmach:master Apr 7, 2020
manuio pushed a commit that referenced this pull request Oct 12, 2020
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.

5 participants