-
-
Notifications
You must be signed in to change notification settings - Fork 200
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
feat: use latest actix, switch to bb8 (async everywhere), remove actions #377
Conversation
Bump actix dependencies to the latest versions The server.rs `fn new(pool, config)` needs more work because the new `HttpServer::new` returns a future that needs to be awaited. I am not an expert in actix, and I'm not certain why `actix::System::new()` was being returned before, and if its still needed. Some help is welcome.
I think we can drop the refresh functionality from the app for now. Let's notify users that this feature was removed when the watch mode cli arg or env var is detected as |
@stepankuzmin thx, good point, removed both. I think eventually we may want to revisit the watch support, so I wonder if it would make sense to remove watch support as a separate PR? this would make it easier to review this PR and easier to revert the watch removal if needed. |
@nyurik yep, that would be more convenient, thanks! |
This reverts commit 920cabe.
@stepankuzmin see #381 - which should be merged before this one. |
@stepankuzmin ok, done with the changes, let me know if this needs any other changes |
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.
Looks good to me 👍
This fixes #349