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

Make Request properties var instead of let #6

Open
fishcharlie opened this issue Jun 13, 2023 · 3 comments
Open

Make Request properties var instead of let #6

fishcharlie opened this issue Jun 13, 2023 · 3 comments
Labels
good first issue Good for newcomers

Comments

@fishcharlie
Copy link
Member

There is no reason the following code should be invalid:

let instance = LemmyAPI(baseUrl: url)

let siteRequest = GetSiteRequest()
siteRequest.auth = jwt

let siteResponse = try await instance.request(siteRequest)

But that code currently produces an error: Cannot assign to property: 'auth' is a 'let' constant.

We should make all those properties var instead of let to allow that code to be valid.

@fishcharlie fishcharlie added the good first issue Good for newcomers label Jun 13, 2023
@RivieraKid
Copy link

As I understand the code, the auth property is intended to be set when instantiating the siteRequest object. This would make sense since the auth method probably shouldn't be able to be changed once set, and it will discourage having unnecessary long-lived objects. That would make your example code this:

let instance = LemmyAPI(baseUrl: url)

let siteRequest = GetSiteRequest(auth: jwt)

let siteResponse = try await instance.request(siteRequest)

I would argue that the issue here is more the lack of documentation around the init method.

@fishcharlie
Copy link
Member Author

@RivieraKid Good perspective. How do you think the documentation should be improved?

@RivieraKid
Copy link

Using Xcode's markdown-like documentation I knocked up a quick example for the init(auth:) method:

    /// Public Initialiser for the GetSiteRequest struct.
    /// - Parameter auth: Authentication token (JWT) to use, or `nil` if absent
    ///
    /// If instantiated with no arguments, the authenticatiion token will be `nil`. This may not be what you want.
    /// To make an authenticated request, include the authentication token:
    /// ```let siteRequest = GetSiteRequest(auth: jwt)```
	public init(auth: String? = nil) {
		self.auth = auth
	}

This appears in Xcode's quick-help pane as follows:
image

I see you have generated API documentation, I assume it will read this kind of documentation and include it in the generated documentation.

FWIW, you can get Xcode to insert boilerplate for these pieces of documentation by typing CMD-Opt-/ when your cursor is on the thing you want to document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants