Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Oct 3, 2024

No description provided.

@ahoppen ahoppen requested a review from shahmishal as a code owner October 3, 2024 23:33
@shahmishal
Copy link
Member

Next time, let’s try to separate formatting and feature work in separate PRs. This makes it easier to review the changes.

@ahoppen
Copy link
Member Author

ahoppen commented Oct 4, 2024

#15 formats the files and #13 now depends on #15.

ahoppen added a commit to ahoppen/swift-format that referenced this pull request Oct 4, 2024
@ahoppen ahoppen force-pushed the yamllint branch 3 times, most recently from a6a8241 to faf6f28 Compare October 4, 2024 16:24
type: boolean
description: "Boolean to enable the YAML lint job. Defaults to true."
default: true
yamllint_container_image:
Copy link
Member Author

Choose a reason for hiding this comment

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

@shahmishal Would you like to have a separate option for the base image of yamllint, spellcheck and flake8? Or should all of this be one single options?

Comment on lines 193 to 194
container:
image: ${{ inputs.yamllint_container_image }}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need a container image at all here we can just run on the runner itself

persist-credentials: false
- name: Run yamllint
run: |
apt-get -qq update && apt-get -qq -y install yamllint
Copy link
Member

Choose a reason for hiding this comment

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

The runner itself comes with yamlint preinstalled so we don't have to install it here

Copy link
Member

@shahmishal shahmishal left a comment

Choose a reason for hiding this comment

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

LGTM!

@shahmishal
Copy link
Member

@FranzBusch, please feel free to merge it once you’ve reviewed it.

@FranzBusch FranzBusch merged commit 66ec6fb into swiftlang:main Oct 10, 2024
@ahoppen ahoppen deleted the yamllint branch October 10, 2024 18:08
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.

3 participants