Skip to content

OpenAPI: use yaml linter - #14686

Merged
kevinjqliu merged 16 commits into
apache:mainfrom
kevinjqliu:kevinjqliu/openapi-lint-yaml
Dec 5, 2025
Merged

kevinjqliu merged 16 commits into
apache:mainfrom
kevinjqliu:kevinjqliu/openapi-lint-yaml

Conversation

@kevinjqliu

@kevinjqliu kevinjqliu commented Nov 25, 2025 •

Copy link
Copy Markdown
Contributor

This PR adds a yaml linter for rest-catalog-open-api.yaml and ../aws/src/main/resources/s3-signer-open-api.yaml to keep formatting consistent, using yamllint to check for formatting issues.

Introduced a few new Makefile commands

make validate-spec
make lint-spec
make lint # runs both above

The linter rules are kept in open-api/.yamllint

@kevinjqliu
kevinjqliu force-pushed the kevinjqliu/openapi-lint-yaml branch from 3185c13 to f272c5c Compare November 28, 2025 22:14
@github-actions github-actions Bot added the AWS label Nov 28, 2025

@Fokko Fokko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks great @kevinjqliu, one comment around the line length

Comment thread open-api/.yamllint

rules:
line-length:
max: 350

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems pretty long?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yea, agreed. im deferring the line-length lint fix for later. want to get the linter installed asap

@github-actions github-actions Bot removed the INFRA label Dec 2, 2025

@geruh geruh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice LGTM! a yaml linter that plays well with the headers

@kevinjqliu

kevinjqliu commented Dec 2, 2025 •

Copy link
Copy Markdown
Contributor Author

All changes to aws/src/main/resources/s3-signer-open-api.yaml and open-api/rest-catalog-open-api.yaml are formatting / cosmetic only.
I tried to address each class of change in individual commits, https://github.com/apache/iceberg/pull/14686/commits

Specifically, here are the list of yamllint errors:
Fixed

  • too few spaces before comment: expected 2 (comments)
  • too few spaces after comma (commas)
  • trailing spaces (trailing-spaces)
  • wrong indentation: expected 16 but found 18 (indentation)
  • too many blank lines (3 > 2) (empty-lines)

Ignored

  • too many spaces inside braces (braces)
  • too many spaces inside brackets (brackets)
  • too many spaces inside empty braces (braces)
  • too many spaces inside empty brackets (brackets)
  • line too long (123 > 120 characters) (line-length)

@kevinjqliu

Copy link
Copy Markdown
Contributor Author

sent out an email to dev@. if there are no objections by EOW, i'll merge this PR

https://lists.apache.org/thread/g4yqzltzs2n4jsdpln66yzz439vy0bpy

@nastra nastra changed the title open-api: use yaml linter OpenAPI: use yaml linter Dec 4, 2025
@kevinjqliu
kevinjqliu merged commit 55bfc7e into apache:main Dec 5, 2025
45 checks passed
@kevinjqliu

Copy link
Copy Markdown
Contributor Author

thanks for the review everyone!

@kevinjqliu
kevinjqliu deleted the kevinjqliu/openapi-lint-yaml branch December 5, 2025 16:39
thomaschow pushed a commit to thomaschow/iceberg that referenced this pull request Jan 19, 2026
talatuyarer pushed a commit to talatuyarer/iceberg that referenced this pull request Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants