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 all error types from awc #1621

Merged
merged 3 commits into from
Aug 14, 2020

Conversation

mwilliammyers
Copy link
Contributor

@mwilliammyers mwilliammyers commented Jul 23, 2020

PR Type

Other

PR Checklist

Check your PR fulfills the following:

  • Tests for the changes have been added / updated. (N/A)
  • Documentation comments have been added / updated. (N/A)
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt

Overview

This makes using error handling while using the awc::Client much easier.

Closes #1474


Or I can update this PR to do one of the following (in order of ascending levels of change):

  1. Import the rest of the missing errors (and the rest of the API as well?) like FrozenClientRequest individually.

  2. Change the error imports to:

    pub use awc::error::*;
  3. Change the whole client module to:

    1. Re-export everything while keeping the current structure:
      pub use awc::error::*;
      pub use awc::*;
    2. Re-export everything to keep the same structure as awc (breaking change):
      pub use awc::*;

@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2020

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1621   +/-   ##
=======================================
  Coverage   53.20%   53.20%           
=======================================
  Files         125      125           
  Lines       11762    11762           
=======================================
  Hits         6258     6258           
  Misses       5504     5504           
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 6e97bc0...7c8ce66. Read the comment docs.

@JohnTitor JohnTitor requested review from a team July 25, 2020 09:35
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

I have no objections for this. Could you also update the changelog?

@mwilliammyers
Copy link
Contributor Author

Should I not add any other imports (or do any of the other options listed above)?

@robjtede
Copy link
Member

robjtede commented Aug 8, 2020

I would be in favour of re exporting all the error types as you mentioned in 2.

@JohnTitor
Copy link
Member

We'd be happy to include this in the next beta release if you could update in a few days :)

@mwilliammyers mwilliammyers changed the title Re-export awc::error::JsonPayloadError Re-export all error types from awc Aug 13, 2020
@mwilliammyers
Copy link
Contributor Author

Done!

I also noticed the change log has a bad link:

[#1618]: https://github.com/actix/actix-web/pull/1610

I can fix that as well, unless you wanted to keep that separate?

@robjtede
Copy link
Member

I can fix that as well, unless you wanted to keep that separate?

might as well since you spotted it 👍

@robjtede robjtede merged commit 5aad8e2 into actix:master Aug 14, 2020
@mwilliammyers mwilliammyers deleted the feat/export-awc-error branch August 14, 2020 03:05
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.

Re-export awc::error::JsonPayloadError
4 participants