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.

Sign up for GitHub

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

Jump to bottom

Correctly handle nil values in arrays #251

Merged
merged 2 commits into from Nov 5, 2021
Merged

Correctly handle nil values in arrays #251

merged 2 commits into from Nov 5, 2021

Conversation

lafriks
Copy link
Contributor

@lafriks lafriks commented Nov 5, 2021

Was to late but posting it here as might be some other cases covered

@lafriks lafriks added the invalid This doesn't seem right label Nov 5, 2021
@codecov
Copy link

codecov bot commented Nov 5, 2021 •
edited

Codecov Report

Merging #251 (66876e3) into master (6aa81d4) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #251   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           56        56           
  Lines         3967      3972    +5     
=========================================
+ Hits          3967      3972    +5
Impacted Files Coverage Δ
collection.go 100.00% <100.00%> (ø)

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 6aa81d4...66876e3. Read the comment docs.

@lafriks lafriks mentioned this pull request Nov 5, 2021
Merged
Fs02
Fs02 reviewed Nov 5, 2021
View reviewed changes
repository_test.go Outdated Show resolved Hide resolved
@lafriks lafriks force-pushed the fix/arr_pointer_insert_delete branch from 5696ab9 to b5594e3 Compare November 5, 2021 12:27
@lafriks
Copy link
Contributor Author

lafriks commented Nov 5, 2021 •
edited

Rebased code, please check TestRepository_Update_ptrElem test for regression - now it panics (previously it did not)

lafriks
lafriks commented Nov 5, 2021
View reviewed changes
document.go Outdated Show resolved Hide resolved
@lafriks lafriks requested a review from Fs02 November 5, 2021 12:40
@lafriks lafriks force-pushed the fix/arr_pointer_insert_delete branch from f5c8118 to 658cc8d Compare November 5, 2021 12:47
Fs02
Fs02 reviewed Nov 5, 2021
View reviewed changes
repository_test.go Outdated Show resolved Hide resolved
document.go Outdated Show resolved Hide resolved
document.go Outdated Show resolved Hide resolved
@lafriks lafriks force-pushed the fix/arr_pointer_insert_delete branch from 55e2653 to ea1169d Compare November 5, 2021 13:56
@lafriks lafriks changed the title Fix array pointer usage for insert and condition primary keys Correctly handle nil values in arrays Nov 5, 2021
@lafriks
Copy link
Contributor Author

lafriks commented Nov 5, 2021

Ok, removed everything except for handling nil values in arrays

@lafriks lafriks added enhancement New feature or request and removed invalid This doesn't seem right labels Nov 5, 2021
Fs02
Fs02 reviewed Nov 5, 2021
View reviewed changes
collection.go Outdated Show resolved Hide resolved
Fs02
Fs02 approved these changes Nov 5, 2021
View reviewed changes
Copy link
Member

@Fs02 Fs02 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 🚀

@Fs02 Fs02 merged commit 30b6827 into go-rel:master Nov 5, 2021
@lafriks lafriks deleted the fix/arr_pointer_insert_delete branch May 16, 2022 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fs02 Fs02 Fs02 approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@lafriks @Fs02