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

fix(extensions/webstorage): use opstate for sqlite connection #10692

Merged
merged 7 commits into from
May 19, 2021
Merged

fix(extensions/webstorage): use opstate for sqlite connection #10692

merged 7 commits into from
May 19, 2021

Conversation

crowlKats
Copy link
Member

@crowlKats crowlKats commented May 18, 2021 •
edited
Loading

Closes #10691

lucacasonato
lucacasonato approved these changes May 19, 2021
View reviewed changes
Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

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

LGTM. Please just run fmt again.

lucacasonato
lucacasonato requested changes May 19, 2021
View reviewed changes
Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

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

Reverting approval because this apparently breaks sessionStorage

lucacasonato
lucacasonato approved these changes May 19, 2021
View reviewed changes
Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

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

Thanks 🙂

@ry
Copy link
Member

ry commented May 19, 2021 •
edited
Loading

I'm confused why this is labeled as "feat". Isn't this a fix?

@crowlKats crowlKats changed the title feat(extensions/webstorage): use opstate for sqlite connection fix(extensions/webstorage): use opstate for sqlite connection May 19, 2021
@crowlKats
Copy link
Member Author

@ry yeah its a fix, my bad

@ry ry merged commit 1cb5ec3 into denoland:main May 19, 2021
@ry
Copy link
Member

ry commented May 19, 2021

Thanks @crowlKats

@crowlKats crowlKats deleted the opstate_webstorage branch May 19, 2021 12:29
piscisaureus pushed a commit that referenced this pull request May 31, 2021
@crowlKats @piscisaureus
fix(webstorage): use opstate for sqlite connection (#10692)
d2c34bc 
Fixes #10691
piscisaureus pushed a commit that referenced this pull request May 31, 2021
@crowlKats @piscisaureus
fix(webstorage): use opstate for sqlite connection (#10692)
4cda688 
Fixes #10691
piscisaureus pushed a commit that referenced this pull request May 31, 2021
@crowlKats @piscisaureus
fix(webstorage): use opstate for sqlite connection (#10692)
7b73841 
Fixes #10691
piscisaureus pushed a commit that referenced this pull request May 31, 2021
@crowlKats @piscisaureus
fix(webstorage): use opstate for sqlite connection (#10692)
35feeff 
Fixes #10691
piscisaureus pushed a commit that referenced this pull request May 31, 2021
@crowlKats @piscisaureus
fix(webstorage): use opstate for sqlite connection (#10692)
a112576 
Fixes #10691
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartlomieju bartlomieju bartlomieju left review comments

@lucacasonato lucacasonato lucacasonato approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: localStorage methods give leaking resource errors in Deno.test
4 participants
@crowlKats @ry @lucacasonato @bartlomieju