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

Fix "Setting Headers" example in "Testing Actions" #1135

Merged
merged 1 commit into from
Dec 28, 2022
Merged

Fix "Setting Headers" example in "Testing Actions" #1135

merged 1 commit into from
Dec 28, 2022

Conversation

pauldps
Copy link
Contributor

@pauldps pauldps commented Dec 27, 2022

Sending two separate arguments fails with an argument error:

Error: wrong number of arguments for 'ApiClient#headers' (given 2, expected 0)

The syntax that works is to send them like a hash:

client.headers("Content-Type": "application/json")

Sending two separate arguments fails with an argument error.
The syntax that works is to send them like a hash: `headers("Content-Type": "application/json")`
Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

Ah, nice catch! Also good to note that you can actually do headers(set_cookie: "...") or header(content_type: "....") since these keys get normalized within Crystal. But we can save that for a future update.

@jwoertink jwoertink merged commit b107924 into luckyframework:main Dec 28, 2022
@pauldps pauldps deleted the patch-1 branch December 28, 2022 00:50
@@ -80,7 +80,7 @@ class Guides::Testing::TestingActions < GuideAction

def page(page : Int32, per_page = 10)
# Set pagination headers
headers("Range", "order,id \#{page * per_page}; order=desc,max=\#{per_page}"
headers("Range": "order,id \#{page * per_page}; order=desc,max=\#{per_page}"
Copy link
Member

Choose a reason for hiding this comment

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

Looks like there's a missing closing paren here as well

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