TheOdinProject / odin-bot-v2

The bot that breathes life into our Discord community
ISC License
48 stars 77 forks source link

Feature: Add prettier GH workflow (--check only) #576

Closed MaoShizhong closed 2 months ago

MaoShizhong commented 3 months ago

Because

Since we have Prettier as a dependency with a .prettierrc file and format npm script, it'd help to have a pull request workflow that checks all changed files for formatting issues (which can be fixed with a simple npm run format -- <paths to files>.

Left at a --check only workflow so any formatting that may need to be done can go in a separate commit for easier reviewing. If a --write workflow would be preferable, let me know.

If merged, I'll update the repo wiki with formatting instructions.

This PR

Issue

Closes #560

Additional Information

Could not find a way to achieve this via CirclCI - could only find out how to get path filtering to allow an action to run on specific file change (and run prettier --check . on everything), rather than an action to only target changed files.

Pull Request Requirements

MaoShizhong commented 3 months ago

Hmmm, looking into why it can't detect a parser/file given it works in a test repo :thinking:

MaoShizhong commented 3 months ago

Turns out ya?ml wasn't valid in the with.files bit, so it ran due to the .yml file but didn't pass it to Prettier :shrug:

MaoShizhong commented 3 months ago

@Asartea Could you post a link here to your local repo where you managed to get it working for changed files only with CircleCI?

Asartea commented 3 months ago

@Asartea Could you post a link here to your local repo where you managed to get it working for changed files only with CircleCI?

See https://github.com/Asartea/top-odin-bot-v2/tree/test-ci for the code (changes made in package.json, prettier-ci.sh, and .circleci/config.yml)

You can see some example runs (both failing and passing) at https://app.circleci.com/pipelines/github/Asartea/top-odin-bot-v2?branch=test-ci (ignore the ones where I made stupid mistakes)

MaoShizhong commented 3 months ago

Thanks. Whoever ends up reviewing this, please let us know whether the GH workflow or CircleCI approach would be preferable. I'm personally not too familiar with CircleCI or bash scripts. If CircleCI would be preferable, then we can close this one superceded by that one.

CouchofTomato commented 3 months ago

@KevinMulhern Can I get your input on this mate and whether CircleCI is viable.

Asartea commented 3 months ago

Also, shopping for a better name for the package.json script and a better filename for the .sh file; I'm unconvinced by the current ones

MaoShizhong commented 1 month ago

@KevinMulhern Do you think it's worth migrating the current circleci workflows to GH actions? From what I can see it runs (jest and eslint), I believe it shouldn't be too difficult to find the appropriate actions for them.

Asartea commented 1 month ago

My two cents is that they should get migrated: its confusing to have workflows split across two different platforms

KevinMulhern commented 1 month ago

Yeah I think it'd be worth the it.

MaoShizhong commented 1 month ago

I'll open an issue for assignment then :)