hotosm / tasking-manager

Tasking Manager - The tool to team up for mapping in OpenStreetMap
https://wiki.openstreetmap.org/wiki/Tasking_Manager
BSD 2-Clause "Simplified" License
510 stars 275 forks source link

Set up pre-commit hook for code quality assurance #5714

Closed Aadesh-Baral closed 3 months ago

Aadesh-Baral commented 1 year ago

I would like to request the implementation of a pre-commit hook in our project's codebase. A pre-commit hook is a script that runs before a developer's changes are committed to version control, allowing us to catch potential issues early and ensure that all commits meet certain quality standards.

By setting up a pre-commit hook, we can ensure that our codebase follows consistent formatting, passes automated tests, and meets other code quality requirements. This will not only make our codebase more readable and maintainable, but also make code reviews more efficient by catching issues before they're even committed.

I propose that we use pre-commit to set up our pre-commit hook. I believe this will be a valuable addition to our development process and help us improve the overall quality of our codebase.

Sample pre-commit config using pre-commit

eternaltyro commented 1 year ago

What hooks do you want added to begin with ? @Aadesh-Baral

Aadesh-Baral commented 1 year ago

Pre-commit hooks for formatting, running unit tests, code linting, and security checks would be helpful. For generating texts for translations, we must also execute yarn build-locale on the frontend, which might also be helpful.

eternaltyro commented 1 year ago

I understand. Can you list out the available plugins to begin with?

Aadesh-Baral commented 1 year ago

I'm not sure about plugins, but I've seen pre-commit used in frameworks like FastAPI.

Aadesh-Baral commented 1 year ago

I think we can use some predefined hooks from pre-commit. (https://pre-commit.com/hooks.html)

spwoodcock commented 1 year ago

A lot of this is bundled with ruff via pre-commit now.

We have it configured for FMTM, but it isn't enforced yet.

I think a good approach could be via pre-commit.ci (free to use in our case) to run on each PR automatically. We should discuss at the Tech Team Meeting!

dakotabenjamin commented 6 months ago

@mahesh-naxa this has been given highest priority for next release

spwoodcock commented 3 months ago

Looks like this is done!

Can it be closed?

mahesh-naxa commented 3 months ago

Can close this.