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

Replace to HTTPTypes Components from Helpers Components #564

Merged

Conversation

zunda-pixel
Copy link
Contributor

@zunda-pixel zunda-pixel commented Oct 14, 2024

Description

HTTP Header

  • Remove Helpers.HTTPHeader
  • Add HTTPTypes.HTTPFields

HTTP Method

  • Remove Helpers.HTTPMethod
  • Add HTTPTypes.HTTPRequest.Method

@grdsdev
Copy link
Collaborator

grdsdev commented Oct 14, 2024

Hi @zunda-pixel thats a nice change, I was waiting for adopting HTTPTypes until they found a way of representing the body too, then I'd also adopt HTTPRequest from HTTPTypes too.

But yeah, I think we can start by migrating the HTTPHeaders, and then migrate HTTPRequest/HTTPResponse in a new PR, or wait until they have the body also represented.

@zunda-pixel zunda-pixel changed the title Replace to HTTPTypes.HTTPFields from Helpers.HTTPHeader Replace to HTTPTypes Components from Helpers Components Oct 14, 2024
@zunda-pixel
Copy link
Contributor Author

Hi! @grdsdev I tried to replace HTTPRequest, but It is hard...
Yes, HTTPTypes.HTTPRequest has no body.
I replaced HTTP Header and HTTP Method

@zunda-pixel zunda-pixel force-pushed the replace-to-HTTPFields-from-HTTPHeader branch 2 times, most recently from 688c3b3 to 686a476 Compare October 14, 2024 11:43
@grdsdev grdsdev self-requested a review October 14, 2024 11:56
@zunda-pixel zunda-pixel marked this pull request as ready for review October 14, 2024 12:14
@@ -51,9 +52,9 @@ public class PostgrestBuilder: @unchecked Sendable {

/// Set a HTTP header for the request.
@discardableResult
public func setHeader(name: String, value: String) -> Self {
public func setHeader(name: HTTPField.Name, value: String) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is breaking change, lets keep the name as a String and create the HTTPField.Name internally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HTTPField.Name.init?(_ name: String) is optional init.
Can we use force unwrap?

@discardableResult
public func setHeader(name: String, value: String) -> Self {
  return self.setHeader(
    name: HTTPField.Name(name)!,
    value: value
  )
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, force unwrap here is fine.

Copy link
Contributor Author

@zunda-pixel zunda-pixel Oct 14, 2024

Choose a reason for hiding this comment

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

ok, I created.

@zunda-pixel zunda-pixel force-pushed the replace-to-HTTPFields-from-HTTPHeader branch from 2e30787 to 1f54086 Compare October 14, 2024 14:03
@grdsdev grdsdev enabled auto-merge (squash) October 14, 2024 14:33
@grdsdev grdsdev merged commit 71dee2a into supabase:main Oct 14, 2024
26 checks passed
@zunda-pixel zunda-pixel deleted the replace-to-HTTPFields-from-HTTPHeader branch October 14, 2024 16:44
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.

2 participants