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

Make start functions return a Result #568

Merged
merged 2 commits into from
Oct 13, 2021

Conversation

msrd0
Copy link
Member

@msrd0 msrd0 commented Oct 6, 2021

Fixes #483

@msrd0 msrd0 added this to the 0.7 milestone Oct 6, 2021
@codecov
Copy link

codecov bot commented Oct 6, 2021

Codecov Report

Merging #568 (ed9c818) into master (0510afa) will increase coverage by 0.08%.
The diff coverage is 27.58%.

❗ Current head ed9c818 differs from pull request most recent head 805d262. Consider uploading reports for the commit 805d262 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #568      +/-   ##
==========================================
+ Coverage   85.14%   85.22%   +0.08%     
==========================================
  Files         111      111              
  Lines        5768     5774       +6     
==========================================
+ Hits         4911     4921      +10     
+ Misses        857      853       -4     
Impacted Files Coverage Δ
examples/cookies/introduction/src/main.rs 88.88% <0.00%> (ø)
examples/diesel/src/main.rs 86.90% <0.00%> (ø)
...les/example_contribution_template/name/src/main.rs 76.19% <0.00%> (ø)
examples/finalizers/src/main.rs 85.71% <0.00%> (ø)
examples/handlers/async_handlers/src/main.rs 91.24% <0.00%> (ø)
examples/handlers/form_urlencoded/src/main.rs 83.33% <0.00%> (ø)
examples/handlers/multipart/src/main.rs 76.19% <0.00%> (ø)
examples/handlers/request_data/src/main.rs 88.88% <0.00%> (ø)
...xamples/handlers/simple_async_handlers/src/main.rs 90.00% <0.00%> (ø)
...s/handlers/simple_async_handlers_await/src/main.rs 90.38% <0.00%> (ø)
... and 29 more

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 0510afa...805d262. Read the comment docs.

@msrd0 msrd0 merged commit 463d3cc into gotham-rs:master Oct 13, 2021
@msrd0 msrd0 deleted the start-return-result branch October 13, 2021 09:04
@FSMaxB
Copy link
Contributor

FSMaxB commented Oct 13, 2021

I'm not entirely sure if it is a good idea to return a std::io::Result here since it doesn't allow for adding additional errors that can be returned. Maybe it's a better idea to introduce an opaque, gotham-specific error type here?

@msrd0
Copy link
Member Author

msrd0 commented Oct 13, 2021

Good point. Probably some non-exhaustive enum that for now just stores io::Error would be better. Do you want to create a PR?

@FSMaxB
Copy link
Contributor

FSMaxB commented Oct 13, 2021

Do you want to create a PR?

Not really at the moment.

msrd0 added a commit to msrd0/gotham that referenced this pull request Oct 13, 2021
This helps future-proofing the API. Thanks @FSMaxB for
suggesting this in gotham-rs#568
msrd0 added a commit that referenced this pull request Oct 20, 2021
This helps future-proofing the API. Thanks @FSMaxB for
suggesting this in #568
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.

start - errors on socket opening
2 participants