Automation to improve code quality and consistency

[shivasankaran18]

Increasing Access

Adding Husky to the p5.js repository would help enforce pre-commit Git hooks for formatting, linting, or running tests, ensuring that contributions from all users—especially beginners—follow a consistent and accepted code style.

Most appropriate sub-area of p5.js?

Home

Feature request details

By automating linting or formatting on commits, contributors don’t need to worry about setting up and remembering project-specific tooling—Husky does it for them.

[shivasankaran18]

@ksen0 @davepagurek I would like to work on this issue ..?

[ksen0]

Hi @shivasankaran18 could you clarify please which specific problems this will help to address? 2-3 examples would be really helpful to understand what your idea here is!

[ksen0]

I also just saw the other issue you opened #738:

Currently, the p5.js-website repository does not have a ts-ignore check in its CI workflow. Adding this check can help ensure that TypeScript code adheres to best practices and avoids the use of ts-ignore comments, which can hide potential issues.

And as a result, I am renaming this issue to more reflect the general proposal of improving code quality/consistency through automation.

Anyone reading is welcome to weight in! What are 2-3 concrete, currently-existing problems that would be addressed by the proposed automation(s)? and from there: which automation is most helpful?

[shivasankaran18]

Yeah @ksen0.. We can have a ts-ignore check script to ensure that modification in the codebase does not include ts-ignore comments .. Adding Husky(Pre Commit Hook) to the repo checks and lints the code for every commit the developer makes .. I propose this , on every commit , we can have linting and ts-ignore check using the pre commit hook . We can also add any other code quality automations if you could suggest
Love to hear your thoughts on this @ksen0

[ksen0]

Hi @shivasankaran18 , thanks for following up! I'm more asking for why, or the motivation; I understand what you're proposing. What are 2-3 concrete, currently-existing problems that would be addressed by the proposed automation(s)? For example, are there any recent commits where these would have been helpful, and how?

EDIT: Sorry I didn't mean to close it just now, clicked the wrong button.

[shivasankaran18]

yeah @ksen0
the problems can be

1. Developers sometimes forget to run the linter before committing, leading to inconsistent formatting
2. Developers use @ts-ignore to suppress TypeScript errors without addressing the root cause. This creates type errors and leads to poor code quality in the code base

[ksen0]

What I'm asking about is not the theoretical problems that might happen, but whether this is an actual problem in the repository. For example, there are a few linter warnings; and use of ts-ignore as a workaround for Image. In my opinion, these are not major existing code quality issues, so even though there can be some improvements, I don't see that either of these topics are currently very big problems. So what I keep asking is whether you've seen recent commits or parts of the code that do look like major code quality problems in the actual codebase - feel free to share your thoughts here.

Each of those suggested changes in automation would need, as prerequisites, to first have 2 respective PRs: one that fixes all existing linter problems; and another that removes need for ts-ignorewithout changing functionality. You are welcome to work on fixing linter problems first. Once that is finished, you could try to remove the existing use of ts-ignore. Let me know if you're interested in working on that!

[shivasankaran18]

yeah got it @ksen0 .. I don't see any recent commits that have major code quality codebase . I am proposing this for future avoidance . I am happy to work on this issue .. could you assign me ?

[ksen0]

Hi @lirenjie95 do you have any thoughts about whether or not to add Husky / other automations for improving code quality and consistency? Pros/cons? Tagging you because of your recent PR comment. Thank you!

[lirenjie95]

Pros: 1. Third-party tools can improve code quality because they have pre-configured patterns. 2. They also save reviewers' time by preventing them from having to point out code styling issues.
Cons: Introducing a dependency for p5.js requires us to check (frequently) if the tool is still maintained by the open-source community.

#820 is a good PR if it only addresses the lint warnings. I agree with @ksen0 that we currently don't have many linter warnings or errors that make this repo hard to maintain.