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

Run Ameba in CI #4753

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Run Ameba in CI #4753

wants to merge 4 commits into from

Conversation

syeopite
Copy link
Member

@syeopite syeopite commented Jun 16, 2024

Ameba has been technically apart of Invidious for a couple years now but its linting was never enforced. This PR changes that.

Following cases such as what was seen in #4709 it'd probably be a good idea to have Ameba check over everything, especially for basic mistakes.

And from the testing I've done its already been pretty helpful. For example a duplicated key in the i18next logic

"pt" => PluralForms::Special_French_Portuguese,
"pt" => PluralForms::Special_French_Portuguese,

This PR simply adds Ameba to the CI but doesn't actually fix any of the detected issues. I'd be opening more PRs in the near future to slowly address everything until Invidious is compliant.

@syeopite syeopite requested a review from a team as a code owner June 16, 2024 21:04
@syeopite syeopite requested review from unixfox and removed request for a team June 16, 2024 21:04
@SamantazFox
Copy link
Member

While you're here, can you bump the ameba version in shard.lock to 1.6.1?
https://github.com/crystal-ameba/ameba/releases

@syeopite
Copy link
Member Author

Once Invidious is Ameba compliant it might be a good idea to switch to https://github.com/crystal-ameba/github-action rather than building and using Ameba ourselves.

This allows the CI to automatically annotate the offending code on Github's UI which is a better experience than manually checking the CI logs.

However, this is a blocker: crystal-ameba/github-action#19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants