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

make change to lib/unset.js #1167

Merged
merged 4 commits into from
Dec 3, 2021
Merged

make change to lib/unset.js #1167

merged 4 commits into from
Dec 3, 2021

Conversation

resession
Copy link
Contributor

@resession resession commented Nov 24, 2021

the .unset() method is not working, i made some changes to my local package of gundb and it seems to be doing something, doesn't give errors, and nulls the data. currently it is the following:
const rel_ = Gun.val.link._; // '#'
const node_ = Gun.node._; // '_'

guessing the Gun.val.link._ was from an old version and it might be breaking now. so i changed it to the following:

const rel_ = '#'; // '#'
const node_ = '_'; // '_'

what do you think?

@amark
Copy link
Owner

amark commented Nov 25, 2021

@resession thanks!

I was planning on deprecating/deleting unset, do you disagree?

Yes, those seem like the correct changes.

@resession
Copy link
Contributor Author

i am currently using unset. the reason actually came from a stackoverflow post i saw from a conversation you had with a person. i can't find the link right now, i searched for it. but the summary is that to delete some data from an unordered list, unset should be used. maybe i am mistaken about what i read, i can't fully recall. maybe you said .set should be used to add data to an unordered list and i am getting it mixed up with deleting with unset.

the main thing is, does it serve a purpose and does it do something that another method can not do? because if so, then keep it(also would help me so i don't have to make changes to gun-fetch and agregor ebrowser doesn't have to update). but if it truly is just another version of .put(null), then you can depracate it/delete it. all up to you.

@resession
Copy link
Contributor Author

since the changes fixes the issue, you can merge this. and then if you want to deprecate or delete the unset module then you can still do that if you are deciding on it.

@amark
Copy link
Owner

amark commented Dec 1, 2021

oops sorry I got busy, I need to try this again.

@sammosna reminded me of this, tho he says he tried it and still not working? (The failing unit test is probably my fault, not @resession ,sometimes the filesystem doesn't clean up after itself on the CI runners which causes it to falsely fail)

@amark
Copy link
Owner

amark commented Dec 3, 2021

Hmm, it keeps timing out. But it looks like things should be fine. If you run mocha locally does it pass?

@amark amark merged commit d1f13c8 into amark:master Dec 3, 2021
@amark
Copy link
Owner

amark commented Dec 3, 2021

Ok it went this time. Pulling! Thanks! :) !!!!! Sorry I'm so slow.

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.

2 participants