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

Strip empty component sections from doc #421

Merged
merged 2 commits into from Apr 7, 2019

Conversation

@lafrech
Copy link
Member

commented Apr 2, 2019

This PR removes empty component subsections (parameters, responses, etc.) from the docs.

I'm tempted not to consider that a breaking change.

@lafrech lafrech requested a review from sloria Apr 6, 2019

@sloria

sloria approved these changes Apr 7, 2019

Copy link
Member

left a comment

Just a little nit. Go ahead and address and merge, or we can address it later.

I wouldn't consider this a breaking change because it's not an API change. This warrants a minor version bump.

@@ -51,7 +51,7 @@ def resolver(schema):
openapi_version="2.0",
plugins=(MarshmallowPlugin(schema_name_resolver=resolver),),
)
assert {} == get_schemas(spec)
assert get_schemas(spec) is None

This comment has been minimized.

Copy link
@sloria

sloria Apr 7, 2019

Member

Nit: It might be good for the get_ functions to default to a sentinel value, like marshmallow.missing.

This comment has been minimized.

Copy link
@lafrech

lafrech Apr 7, 2019

Author Member

I'd rather avoid missing as this could pass a wrong change leaking the missing object into the generated spec.

I could create a dedicated MISSING_FROM_SPEC singleton.

Overall, it looks just as simple to catch KeyError. Also, when using OAS3, KeyError covers the case where the whole components section is missing. See new commit.

This comment has been minimized.

Copy link
@sloria

sloria Apr 7, 2019

Member

Works for me.

@sloria

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

Is there a corresponding issue for this PR? I thought I remember seeing one but I can't find it.

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2019

Just a comment: #408 (comment).

@lafrech lafrech merged commit 14b4762 into dev Apr 7, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@lafrech lafrech deleted the strip_empty_subsections_from_docs branch Apr 7, 2019

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2019

Merged. Thanks for reviewing.

@sloria

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

No prob. Will you take care of the release, or shall I do that?

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2019

I can. Never did. Do I just need to push a tag?

@sloria

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

Yeah, pretty much. The process is the same as marshmallow's: https://github.com/marshmallow-code/marshmallow/blob/dev/RELEASING.md

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2019

Thanks. I never noticed this file.

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Apr 8, 2019

Done.

I added release notes on Tidelift for 1.1.0 and 1.1.1. 1.1.2 and 1.2.0 are not there yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.