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

Inner field of web::Query is public again (#2016) #2017

Merged
merged 2 commits into from
Feb 20, 2021

Conversation

adamchalmers
Copy link
Contributor

@adamchalmers adamchalmers commented Feb 20, 2021

PR Type

Feature

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

The v4 beta made Query's inner field private, but the discussion in the linked issue said that we should revert this change, as it doesn't prevent any bad behaviour.

Because none of the documentation or examples have mentioned using .0 syntax, I didn't think any documentation changes were necessary. I didn't think any tests were necessary either, as this doesn't add any logic, it merely exports a field. If you'd like me to add an example or test, I'd be happy to!

Closes #2016

@robjtede
Copy link
Member

robjtede commented Feb 20, 2021

Would appreciate a doc comment/test. You can edit the current one to use the public field if you like.

And no need for a changelog entry since this is a beta revert.

@adamchalmers
Copy link
Contributor Author

OK, added doctests to demonstrate how to use .0 or .into_inner.

CHANGES.md Outdated Show resolved Hide resolved
Copy link
Member

@robjtede robjtede left a comment

Choose a reason for hiding this comment

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

LGTM

@robjtede robjtede merged commit 2dbdf61 into actix:master Feb 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web project: actix-web B-semver-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider making web::Path and web::Query fields public again
2 participants