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 a linter for rack.early_hints #1695

Closed
wants to merge 1 commit into from

Conversation

casperisfine
Copy link
Contributor

Fix: #1692

This is kind of a first stab, I'm not familiar enough with Rack::Lint to understand exactly what's needed. Please let me know if there are specifications I missed, or if the main description should be improved.

cc @jeremyevans

lib/rack/lint.rb Outdated Show resolved Hide resolved
lib/rack/lint.rb Outdated Show resolved Hide resolved
@ioquatix
Copy link
Member

I'm okay with this, but I think we should defer to the response headers rather than re-specifying it to avoid duplicating the specifications.

@jeremyevans
Copy link
Contributor

With the changes to SPEC since this was originally submitted, I think this will require significant changes before merging. rack.early_hints should be called with an argument that would be valid as response headers. The Lint implementation can call check_headers with the value.

@casperisfine Unless you want to work on this, I can see if I can update it tomorrow.

@casperisfine
Copy link
Contributor Author

I don't absolutely want to work on it, feel free to take over if you wish, but otherwise I can probably do that in a day or two.

jeremyevans added a commit to jeremyevans/rack that referenced this pull request Mar 16, 2022
This is already de facto spec as both Unicorn and Puma
implement it. The changes to SPEC are compatible with
both implementations.

Fixes rack#1692
Fixes rack#1695

Co-authored-by: Jeremy Evans <code@jeremyevans.net>
@casperisfine
Copy link
Contributor Author

Sorry for the delay, I rebased and replaced the content of check_early_hints by check_headers.

It's been a while, so I hope the result is decent. Again feel free to directly take over if you'd rather quickly improve it rather than go through review rounds.

@jeremyevans
Copy link
Contributor

@casperisfine Sorry for not being more explicit about it, but I already submitted a follow-up PR that includes specs: #1831

@casperisfine
Copy link
Contributor Author

Ah that great!

jeremyevans added a commit to jeremyevans/rack that referenced this pull request Aug 14, 2022
This is already de facto spec as Unicorn, Puma, and Falcon
implement it. The changes to SPEC are compatible with
both implementations.

Fixes rack#1692
Fixes rack#1695

Co-authored-by: Jeremy Evans <code@jeremyevans.net>
jeremyevans added a commit to jeremyevans/rack that referenced this pull request Aug 27, 2022
This is already de facto spec as Unicorn, Puma, and Falcon
implement it. The changes to SPEC are compatible with
both implementations.

Fixes rack#1692
Fixes rack#1695

Co-authored-by: Jeremy Evans <code@jeremyevans.net>
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.

Add spec for early hints
4 participants