-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New feature : linting #2557
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
base: master
Are you sure you want to change the base?
New feature : linting #2557
Conversation
Add spec All specs are now linted
Should we call it But thinking about it, maybe we need something more generic that allows one to inject middleware here, including potentially the ability to remove |
Add `yaml-dev` in Dockerfile
The middleware stack lacks a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please document this in the README if you want it merged.
That said, also consider not merging this and doing a configurable middleware implementation instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new linting feature into the Grape framework, allowing endpoints to be linted by default or via a per-endpoint configuration. Key changes include:
- Enabling the global lint configuration in spec_helper and lib/grape.
- Incorporating Rack::Lint middleware in the endpoint stack when linting is enabled.
- Adding a new DSL method lint! to allow per-endpoint lint configuration.
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
spec/spec_helper.rb | Enables linting globally by setting Grape.config.lint to true. |
spec/integration/rails/mounting_spec.rb | Modifies Rails app instantiation and wraps the app with Rack::Lint. |
spec/grape/api_spec.rb | Adds tests for the new .lint! behavior and Rack::Lint error handling. |
lib/grape/endpoint.rb | Inserts Rack::Lint middleware conditionally and defines a lint? method. |
lib/grape/dsl/routing.rb | Introduces lint! in the DSL for endpoint-specific lint enabling. |
lib/grape.rb | Sets the default lint configuration to false. |
Files not reviewed (1)
- docker/Dockerfile: Language not supported
I like the insert_before Grape::Middleware::Error, Rack::Lint I think the |
5d45776
to
fad6b2e
Compare
Fix typo in UPGRADING.md
fad6b2e
to
ed493d0
Compare
This PR adds a linting feature that can activated through
Grape.config.lint = true
orlint!
in aGrape::Api
.Suggested in this comment.
By design, Grape's CI has
Grape.config.lint = true
, so all specs calling an endpoint are linted.