Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

Add db.getMany(keys) #184

Merged
merged 9 commits into from
Nov 27, 2021
Merged

Conversation

MeirionHughes
Copy link
Member

@MeirionHughes MeirionHughes commented Oct 9, 2021

#183

@vweevers I've never actually cherry picked (in like 15 years) - this seemed to have no issues being picked and applied. So you want them all in one, or one PR at a time?

@vweevers
Copy link
Member

vweevers commented Oct 9, 2021

One PR is fine with me; just keep the individual commits.

@MeirionHughes MeirionHughes marked this pull request as draft October 9, 2021 10:23
@MeirionHughes MeirionHughes changed the title Cherry Pick: Close database on environment exit Add db.getMany(keys) Oct 9, 2021
@MeirionHughes MeirionHughes marked this pull request as ready for review October 9, 2021 13:58
@MeirionHughes MeirionHughes requested a review from vweevers October 9, 2021 13:58
@vweevers
Copy link
Member

vweevers commented Oct 9, 2021

Did you have any conflicts when cherry-picking?

@MeirionHughes
Copy link
Member Author

once I realised I needed b4085f0, there was nothing nasty:

an extra env_ needed fixing (55547fa),
hand fix the const keyword additions (8444933).

I opted to keep the entire head version of README.MD (rocksdb) as its completely different (968cfcb)

@vweevers
Copy link
Member

vweevers commented Oct 9, 2021

once I realised I needed b4085f0, there was nothing nasty

Good catch.

Copy link
Member

@vweevers vweevers left a comment

Choose a reason for hiding this comment

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

I'll want to merge this with a "Rebase and merge", meaning all commits of this PR will go into master. Can you replace the broken leveldown references in the commit messages with links to either leveldown commits or PRs? Taking 84a05e8 as an example, change its message from:

Close database on environment exit (#783)
Closes #667, closes #755.

To:

Close database on environment exit

Ported from https://github.com/Level/leveldown/pull/783

Some examples from rocksdb history: 891bb7a, 69e3dcb, 5eee577. The format doesn't matter much, just that we'll know where these rocksdb commits came from.

@MeirionHughes
Copy link
Member Author

MeirionHughes commented Oct 9, 2021

I picked them into a seperate branch and amended each with their original sha's from levelup, then forced that branch back to this PR. That okay? on the PR they're showing up but github isn't making them a link for me.

I guess it needs an actual http link... ? sigh lol. I'll do it again

@vweevers
Copy link
Member

vweevers commented Oct 9, 2021

They're cross-repo references, so for them to be auto-linked by GitHub, commits require the format Level/leveldown@xxx, PRs require the format Level/leveldown#123 and yeah, HTTP URLs are good too.

Because this uses an iterator under the hood, it also refactors
shared code between `db.iterator()` and `db.clear()`.

(cherry picked from commit Level/leveldown@aedf49e)
By using `emplace_back()`, reusing the `std::vector` cache between
`iterator.next()` calls, and not advancing the iterator prematurely.
That last one only matters for single reads (i.e. the first `next()`
call or one made after seeking) and it doesn't improve performance
compared to the previous release, just undoes a mistake in Level/leveldown@b815bea.

(cherry picked from commit Level/leveldown@112906b)
And add `const` and `private` where appropriate.

(cherry picked from commit Level/leveldown@576d135)
@vweevers
Copy link
Member

vweevers commented Oct 9, 2021

You might like git's interactive rebase, allows you to edit commit messages (and more) on the same branch.

@MeirionHughes
Copy link
Member Author

You might like git's interactive rebase, allows you to edit commit messages (and more) on the same branch.

hu... I didn't know you could do that. It shows up on tortoise git if I try to rebase the branch to an earlier point and select Force Rebase.

Well they should all have links now. hopefully...

Copy link
Member

@vweevers vweevers left a comment

Choose a reason for hiding this comment

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

Great work!

@vweevers vweevers added the semver-minor New features that are backward compatible label Oct 9, 2021
@vweevers vweevers self-assigned this Nov 21, 2021
@vweevers vweevers merged commit e6ae598 into Level:master Nov 27, 2021
@vweevers vweevers mentioned this pull request Nov 27, 2021
4 tasks
@vweevers
Copy link
Member

5.2.0

@MeirionHughes MeirionHughes deleted the cherry-leveldown-783 branch November 29, 2021 09:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
semver-minor New features that are backward compatible
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants