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

DEV: add ruby linting steps to linting job in theme workflow #155

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

tyb-talks
Copy link
Contributor

@tyb-talks tyb-talks commented Oct 16, 2024

This adds steps to lint ruby files in the theme workflow. Dependent on discourse/DiscoTOC#97.

@tyb-talks
Copy link
Contributor Author

Will leave this in place till all themes are updated by the update-rb-linting script so their CI won't fail.

@davidtaylorhq
Copy link
Member

davidtaylorhq commented Oct 16, 2024

@tyb-talks this workflow needs to remain functional for third-party themes which don't have Ruby linting config. So maybe we should gate these new steps on the presence of the relevant config files? e.g. the stree config, and the rubocop config?

- name: Rubocop
if: ${{ !cancelled() }}
run: |
if test -f .rubocop.yml; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we could use hashFiles in the if check above (it should return empty string when there aren't matching files) instead of this, but this follows how we've set out these checks elsewhere. @davidtaylorhq

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 mind either way

@tyb-talks tyb-talks merged commit e644b12 into main Oct 18, 2024
10 checks passed
@tyb-talks tyb-talks deleted the dev-add-rb-linting-to-theme-workflow branch October 18, 2024 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants