-
Notifications
You must be signed in to change notification settings - Fork 198
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
migrate crate-details, match_version, db:: and test-fakes to sqlx #2325
Conversation
@rust-lang/docs-rs-reviewers any chance for a review? I know it's much code, but I would love to push this forward. After these async refactors I could then invest some time into making our web handlers much more readable / understandable. |
I can try but my review won't be up to much as I don't much this part of the code. |
.sqlx/query-007a6d355f893370b544dda0d6129096357aa95c67c76905a6709a66247450ff.json
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the strange JSON files, this is just a crate change so nothing to note in particular from me.
It's a little more than a crate change, since many methods get converted from sync to async. But generally yes ;) |
Yes absolutely, but it didn't seems strange or anything. So please just add a readme file in the json folder so people like me will know what's going on in there and why it even exists in the first place. :) |
Finding a working smaller piece to migrate became harder and harder, this is bigger than I hoped, but works.
This migrates some major parts to sqlx & async, also including parts used in our main
web::rustdoc
endpoints.Namely:
CrateDetails
andmatch_version
, and the remaining release-details / list endpointsdb::add_package
methods,db::file
test::fakes
is async internally with sync wrapper methods for old testsFor things I didn't want to migrate (yet) I had to sprinkle plenty of
runtime.block_on
around to make it work.I think there is a nicer way to design the whole
TestEnvironment
/FakeRelease
story, perhaps with having async versions of both classes, similar toAsyncStorage
/Storage
. I would leave fixingAlso includes the changes from #2324