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

Make the github action linter step fail in the case of issues #374

Merged
merged 2 commits into from
Feb 17, 2024

Conversation

julienw
Copy link
Contributor

@julienw julienw commented Feb 15, 2024

I noticed that the github action runs the formatter, but doesn't check that anything was actually reformatted as a result. So it was always successful.

To fix the issue, my strategy is to run a special form of git status whose output won't change over time. If the output is empty, this means that there's no change. Otherwise the action outputs some helpful diagnostics.

Here are examples of output:
Failing: https://github.com/WebKit/Speedometer/actions/runs/7920439286/job/21623479161?pr=374
Successful: https://github.com/WebKit/Speedometer/actions/runs/7920372392/job/21623262244?pr=374

@julienw julienw force-pushed the fix-linter-in-ci branch 3 times, most recently from 93f67b1 to 6dfb051 Compare February 15, 2024 17:44
@julienw julienw marked this pull request as draft February 15, 2024 17:44
@julienw julienw force-pushed the fix-linter-in-ci branch 4 times, most recently from fe25be4 to 4389731 Compare February 15, 2024 18:00
@julienw julienw force-pushed the fix-linter-in-ci branch 2 times, most recently from 46174ab to a41f0e0 Compare February 15, 2024 18:13
@julienw julienw marked this pull request as ready for review February 15, 2024 18:31
@rniwa rniwa added the trivial change A change that doesn't affect benchmark results label Feb 16, 2024
@julienw julienw merged commit ca9d23c into WebKit:main Feb 17, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
trivial change A change that doesn't affect benchmark results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants