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

Add ref missing parameter in content endpoint #58

Merged
merged 2 commits into from
Feb 6, 2017
Merged

Conversation

Palleas
Copy link
Collaborator

@Palleas Palleas commented Feb 4, 2017

The current API doesn't allow you to get a content at a specific ref and this fixes it.

@Palleas Palleas changed the title Add ref missing parameter in content Add ref missing parameter in content endpoint Feb 4, 2017
Copy link
Owner

@mdiep mdiep left a comment

Choose a reason for hiding this comment

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

Is there a way that we can test this? Maybe testing that the Endpoint has the correct parameters? Or maybe that's only worth doing if we need to special case a nil ref.

return "/repos/\(owner)/\(repository)/contents/\(path)"
}
}

internal var queryItems: [URLQueryItem] {
return []
switch self {
case let .content(_, _, _, ref): return [URLQueryItem(name: "ref", value: ref)]
Copy link
Owner

Choose a reason for hiding this comment

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

Does this behave correctly if ref is nil?

@Palleas
Copy link
Collaborator Author

Palleas commented Feb 5, 2017

Rebased and Added some tests for the Endpoint enum. ✌🏼

public func create(file: File, atPath path: String, in repository: Repository) -> SignalProducer<(Response, FileResponse), Error> {
return send(file, to: .content(owner: repository.owner, repository: repository.name, path: path), using: .put)
/// Create a file in a repository
public func create(file: File, atPath path: String, in repository: Repository, in branch: String? = nil) -> SignalProducer<(Response, FileResponse), Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

These new parameters above should follow the API Design Guidelines: https://swift.org/documentation/api-design-guidelines/

Compensate for weak type information to clarify a parameter’s role.

@Palleas
Copy link
Collaborator Author

Palleas commented Feb 6, 2017

@ikesyo is it better now? I used inBranch and atRef 🤔

@mdiep mdiep merged commit a27d190 into master Feb 6, 2017
@mdiep mdiep deleted the get-file-at-ref branch February 6, 2017 02:45
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.

3 participants