devedmonton / DES-Website

The Dev Edmonton Society website! We empower Edmonton Developers!
https://devedmonton.com
MIT License
22 stars 52 forks source link

`npm run pr-checks` should be run when I push a commit #265

Closed MandyMeindersma closed 10 months ago

MandyMeindersma commented 11 months ago

make some sort of git hook

DerrykBoyd commented 11 months ago

@MandyMeindersma can you assign this to me? I'd like to explore setting this up at a github action

DerrykBoyd commented 11 months ago

Actually no need to github actions, I realised we were already checking the linting and tests on netlify as part of the build process, so I can add the formatter there as well easily. #266 does that

DerrykBoyd commented 11 months ago

Sorry for hijacking 😅 just noticed the simple fix so went ahead.

MandyMeindersma commented 11 months ago

oh but I was thinking it's probably helpful to do it as a pre push git hook. I still might do that? keeps the pr's cleaner?

DerrykBoyd commented 11 months ago

oh but I was thinking it's probably helpful to do it as a pre push git hook. I still might do that? keeps the pr's cleaner?

Yeah that's an option. I'd be a little concerned about first time contributors not knowing how the git hooks / formatter works and not being able to get support. With an open PR it would be easier for someone to coach them through that

I would probably also only run the format check on pre push, I think it's a valid workflow to push up some code with lint / test errors with the intention of collaborating or coming back to fix.

MandyMeindersma commented 11 months ago

Maybe add a note in the readme or contributing guidelines about --no-verify?

Would that be a happy compromise? hahah

DerrykBoyd commented 11 months ago

Yup for sure. 😀

MandyMeindersma commented 10 months ago

ah okay based on the last pr I helped with, you're right. Having them done in the pr is better