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 bug https://github.com/ORIGYN-SA/CanDB/issues/28 #30

Draft
wants to merge 4 commits into
base: beta
Choose a base branch
from

Conversation

vporton
Copy link

@vporton vporton commented Mar 1, 2024

This is an alleged fix of the bug #28. But it seems that the fix does not work :-(

Please, help with testing.

@vporton
Copy link
Author

vporton commented Mar 1, 2024

Now it is tested by https://github.com/vporton/bug-candb-delete well. But we need to add automatic tests (with all variants: adding an attribute to an empty value, adding an attribute to a value with another attribute, adding an attribute to a value having the same-named attribute. We also should test when all attributes are removed.

A comment in the submission shows a discrepancy between the documented behavior and real behavior of RT.update. This needs to be cleansed (and possibly can speedup code). I don't quite understand how your tests work.

My code is sub-optimal in cycles used. Please, speedup it. But most importantly, test all variants.

@vporton vporton changed the title [WIP] Fix bug https://github.com/ORIGYN-SA/CanDB/issues/28 Fix bug https://github.com/ORIGYN-SA/CanDB/issues/28 Mar 1, 2024
@vporton
Copy link
Author

vporton commented Mar 1, 2024

I've removed WIP from the title of the PR, because it is a critical issue and tests seems to have it worked. Right now, we can ignore that it has suboptimal speed, more important that it fixes a critical bug.

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.

1 participant