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

re-export actix_rt::main macro #1559

Merged
merged 3 commits into from
Jun 18, 2020
Merged

re-export actix_rt::main macro #1559

merged 3 commits into from
Jun 18, 2020

Conversation

robjtede
Copy link
Member

@robjtede robjtede commented Jun 9, 2020

Re-exports actix_rt::main as actix_web::main (and actix_rt::test as actix_web::test_rt since test is already a module). Also bumps minimum rt version to 1.1.1.

Would like to hear thoughts on this. Seems advantageous to remove the requirement for users to add actix-rt as a direct dependency. The question gets asked reasonably often of why two deps are needed for the hello world example.

@robjtede robjtede requested review from a team June 9, 2020 23:33
@cetra3
Copy link
Contributor

cetra3 commented Jun 10, 2020

I think that makes sense, but would need to update documentation for it. Is there any reason why actix_rt would be used besides the macro? I don't think I've used it directly in my code.

@robjtede
Copy link
Member Author

robjtede commented Jun 10, 2020

Using actix_rt::spawn is probably the second most common case for the crate. Then the test macro (I'm not completely sold on re-exporting it like this, if there are objections speak up 😄). If this idea is accepted would update docs, website and examples.

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2020

Codecov Report

Merging #1559 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1559   +/-   ##
=======================================
  Coverage   51.03%   51.03%           
=======================================
  Files         129      129           
  Lines       12049    12049           
=======================================
  Hits         6149     6149           
  Misses       5900     5900           
Impacted Files Coverage Δ
src/lib.rs 68.42% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ba73fc...144535d. Read the comment docs.

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

Sounds like a good idea to me.

Copy link
Contributor

@cetra3 cetra3 left a comment

Choose a reason for hiding this comment

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

Approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants