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

Audit - Request ACL #2804

Merged
merged 1 commit into from Apr 12, 2019

Conversation

Projects
None yet
2 participants
@cnoon
Copy link
Member

commented Apr 12, 2019

Update the ACL on all Request classes to be public instead of open.

Goals ⚽️

Remove the ability to subclass the Request types.

Implementation Details 🚧

AF5 supports extensibility and customization through composition instead of inheritance. Because of this, we've decided to limit subclassing to Alamofire types only (DataRequest, DownloadRequest, UploadRequest). We are limiting the ability to subclass these types since we do not want to expose the complexities of the internal request system through public APIs. Without those, you wouldn't be able to do much anyways by subclassing. The addition of the event monitoring system in AF5 as well removes the need to subclass purely for proxying API calls.

Testing Details 🔍

N/A

@cnoon cnoon added the request label Apr 12, 2019

@cnoon cnoon added this to the 5.0.0-beta.5 milestone Apr 12, 2019

@cnoon cnoon requested a review from jshier Apr 12, 2019

@cnoon cnoon self-assigned this Apr 12, 2019

@jshier

jshier approved these changes Apr 12, 2019

Copy link
Contributor

left a comment

👍, pending final.

@cnoon

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2019

Chatted offline in Slack that final doesn't make sense since you can't unfinal with @testable, and it is really useful to be able to subclass for mocking certain behaviors in test suites.

@cnoon cnoon merged commit dc836f6 into master Apr 12, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@cnoon cnoon deleted the audit/request-acl branch Apr 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.