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

Path: make content field pub again #2160

Closed
wants to merge 1 commit into from
Closed

Conversation

abonander
Copy link

@abonander abonander commented Apr 13, 2021

PR Type

Refactor

PR Checklist

  • 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.
  • (Team) Label with affected crates and semver status.

Overview

Upgrading to the beta and this seems to have changed from 3.3.5 but no reason is given in #1894 for the change. The content field of Json is still public which confuses me as to why Path and Query got this treatment but Json did not.

I recently switched from calling .into_inner() to destructuring in the function arguments and it's a lot nicer, especially for singular path arguments such as:

#[get("/user/{id}")]
async fn get_user(Path(user_id): Path<Uuid>) -> Option<User> {
    // ...
}

I really don't want to have to go back to writing .into_inner() again. It's just noise.

Upgrading to the beta and this seems to have changed from `3.3.5` but no reason is given in actix#1894 for the change so I presume it was a mistake. The content field of `Json` is still `pub` which supports this conclusion.

I recently switched from calling `.into_inner()` to destructuring in the function arguments and it's a lot nicer, especially for singular path arguments such as:

```rust
#[get("/user/{id}")]
async fn get_user(Path(user_id): Path<Uuid>) -> Option<User> {
    // ...
}
```
@abonander
Copy link
Author

abonander commented Apr 13, 2021

@robjtede I see the discussion in #2016 but I don't think making the field private was necessary, it can just be a style suggestion in the docs to not access tuple fields directly. Paths with multiple parameters should probably switch to using structs anyway.

@robjtede
Copy link
Member

Sorry I'm not considering this change at this time. The discussion in #2016 remains true.

@robjtede robjtede closed this Apr 22, 2021
@abonander abonander deleted the patch-1 branch August 4, 2021 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants