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

extract more config types from Data<T> as well #1641

Merged
merged 1 commit into from Sep 2, 2020
Merged

Conversation

robjtede
Copy link
Member

@robjtede robjtede commented Aug 21, 2020

PR Type

Feature

PR Checklist

Check your PR fulfills the following:

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

Overview

Follow up to #1610. Extracts Form and Json config objects as T and then tries Data<T> as well.

@robjtede robjtede added C-improvement Category: an improvement to existing functionality B-semver-minor labels Aug 21, 2020
@robjtede robjtede mentioned this pull request Aug 21, 2020
15 tasks
@robjtede robjtede marked this pull request as ready for review August 21, 2020 22:15
@robjtede robjtede force-pushed the permissive-config branch 2 times, most recently from 029edda to 500a038 Compare August 21, 2020 22:21
@robjtede robjtede requested review from a team August 21, 2020 22:24
@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2020

Codecov Report

Merging #1641 into master will increase coverage by 0.09%.
The diff coverage is 97.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1641      +/-   ##
==========================================
+ Coverage   53.07%   53.17%   +0.09%     
==========================================
  Files         122      126       +4     
  Lines       11803    11836      +33     
==========================================
+ Hits         6265     6294      +29     
- Misses       5538     5542       +4     
Impacted Files Coverage Δ
src/types/form.rs 81.81% <93.75%> (+2.53%) ⬆️
src/types/json.rs 90.82% <100.00%> (+0.76%) ⬆️
src/types/payload.rs 86.30% <100.00%> (-0.17%) ⬇️
actix-http/src/header/common/accept.rs 0.00% <0.00%> (ø)
actix-http/src/header/common/accept_charset.rs
actix-http/src/header/common/expires.rs 0.00% <0.00%> (ø)
...ctix-http/src/header/common/if_unmodified_since.rs 0.00% <0.00%> (ø)
actix-http/src/header/common/if_modified_since.rs 0.00% <0.00%> (ø)
actix-http/src/header/common/accept_language.rs 0.00% <0.00%> (ø)
actix-http/src/header/common/content_language.rs 0.00% <0.00%> (ø)

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 01cbef7...e92c0dd. Read the comment docs.

@robjtede
Copy link
Member Author

@actix/contributors could use your help to get v3 over the last couple hurdles :D

Copy link
Contributor

@fakeshadow fakeshadow left a comment

Choose a reason for hiding this comment

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

looks great to me and I like this approach than a single type of app state

src/types/json.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-semver-minor C-improvement Category: an improvement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants