Skip to content

Added a second argument for head method#286

Open
numice wants to merge 2 commits into
rage-rb:mainfrom
numice:head_method_arg
Open

Added a second argument for head method#286
numice wants to merge 2 commits into
rage-rb:mainfrom
numice:head_method_arg

Conversation

@numice
Copy link
Copy Markdown
Contributor

@numice numice commented May 7, 2026

Added a second argument for head method in controller/api.rb

@numice
Copy link
Copy Markdown
Contributor Author

numice commented May 7, 2026

I'm not exactly sure if that's right and if some tests are needed. On the first look, I thought that was going to involve many changes. Then I got busy for awhile. Now I took a look again and that's how I understand the task is. Well, it's just one change. But I don't know if anything else is supposed to be added.

@rsamoilov
Copy link
Copy Markdown
Member

Hi @numice

Happy to see you back!

This is a good start. The next step would be to implement a code that will merge options (which is essentially a hash holding response headers the user wants to set) into the response headers that the framework will pass back to Rack.

@numice
Copy link
Copy Markdown
Contributor Author

numice commented May 12, 2026

@rsamoilov Got it. My bad I didn't look at the source code in Rails close enough to get this.

@numice numice force-pushed the head_method_arg branch from 6c77aae to c8a6c81 Compare May 20, 2026 23:43
@numice
Copy link
Copy Markdown
Contributor Author

numice commented May 20, 2026

@rsamoilov Hi. Sorry for the delay. I've updated it based on my understanding. Not totally sure if this is it.

Copy link
Copy Markdown
Member

@rsamoilov rsamoilov left a comment

Choose a reason for hiding this comment

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

That's it!

How are you feeling about covering this with tests? I'll be happy to merge once there's test coverage for the change.

Let me know if you need any help.

@numice
Copy link
Copy Markdown
Contributor Author

numice commented May 23, 2026

Thank you for taking a look at it. I was actually thinking about adding a test but not entirely sure how to go with it. I will take a look at the existing tests and then try to come up with something.

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