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.

Sign up for GitHub

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

Jump to bottom

feat(layered-include_in_schema): Add include_in_schema option for all layers #2295

Merged
merged 7 commits into from
Sep 12, 2023
Merged

feat(layered-include_in_schema): Add include_in_schema option for all layers #2295

merged 7 commits into from
Sep 12, 2023

Conversation

Alc-Alc
Copy link
Contributor

@Alc-Alc Alc-Alc commented Sep 11, 2023

Add include_in_schema option for all layers

Pull Request Checklist

  • New code has 100% test coverage
  • (If applicable) The prose documentation has been updated to reflect the changes introduced by this PR
  • (If applicable) The reference documentation has been updated to reflect the changes introduced by this PR

Description

Close Issue(s)

@Alc-Alc Alc-Alc requested review from a team as code owners September 11, 2023 08:38
@Alc-Alc Alc-Alc force-pushed the 2267-enhancement-add-include_in_schema-option-on-routers-andor-controllers branch from 1cf19ca to ac342a7 Compare September 11, 2023 08:38
@v3ss0n
Copy link
Contributor

v3ss0n commented Sep 11, 2023 •
edited
Loading

include_in_schema sounds a bit long and duplicated .
since it is a Flag , how about in_schema or in_openapi

@guacs
Copy link
Member

guacs commented Sep 11, 2023

include_in_schema sounds a bit long and duplicated . since it is a Flag , how about in_schema or in_openapi

In the decorators, the argument's name is include_in_schema so it's probably best to keep it the same in this case as well for consistency.

@v3ss0n
Copy link
Contributor

v3ss0n commented Sep 11, 2023

include_in_schema sounds a bit long and duplicated . since it is a Flag , how about in_schema or in_openapi

In the decorators, the argument's name is include_in_schema so it's probably best to keep it the same in this case as well for consistency.

okay.

Goldziher
Goldziher reviewed Sep 12, 2023
View reviewed changes
Copy link
Contributor

@Goldziher Goldziher left a comment

Choose a reason for hiding this comment

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

All in all very good, a few comments

litestar/handlers/http_handlers/base.py Outdated Show resolved Hide resolved
litestar/handlers/http_handlers/base.py Outdated Show resolved Hide resolved
litestar/handlers/http_handlers/decorators.py Outdated Show resolved Hide resolved
litestar/handlers/http_handlers/decorators.py Outdated Show resolved Hide resolved
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@Alc-Alc Alc-Alc merged commit aaf3fac into main Sep 12, 2023
@Alc-Alc Alc-Alc deleted the 2267-enhancement-add-include_in_schema-option-on-routers-andor-controllers branch September 12, 2023 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Goldziher Goldziher Goldziher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhancement: Add include_in_schema option on Routers and/or Controllers
4 participants
@Alc-Alc @v3ss0n @guacs @Goldziher