Adding husky precommit tests and eslint config

[akshatnema]

Reason/Context

This is my idea to propose some addition of tests in our project. This may be wrong in your perspective. Do tell me in the discussion, what is best according to you.

• Why we need this improvement? So far I worked in this repository, I saw that many code files have been formatted using different code formatters. So, this has created a type of non-uniformity in the code. And also, there are no tests available so far for the contributors to run their changes on some tests.

• How will this change help? Right now, I haven't made any plan for adding custom test scripts. Rather, I suggested previously to have lint command running in our npm scripts using eslint. Fran has already made a configuration of eslint in the repository. Now, we have to connect it with scripts and tests.

• What is the motivation? I have done this in my projects so far, and this gives me good results. Any newcomers, who contribute to our projects, don't have to worry much about whether it will run or fail in production. Tests results help a lot in solving errors.

Description

• What changes have to be introduced? I will be adding 2 more DevDependencies in our project named husky and prettier. husky npm package will help me to setup a pre-commit command in our project. It will be a npm run test command which will be automatically run in the terminal, as soon as a contributor makes a commit. prettier npm package as a code formatter for the project so as all the files (including md files) get formatted using this package easily. The scripts of both packages will be implemented in package.json file.

• Will this be a breaking change? It will not be a breaking change, but sure, It will be a big change in the codebase and workflow. If we implement any code formatter, like Prettier, it will sure make a change of around a thousand lines in the first commit to format the code. It may happen that contributors face a lot of conflicts in their active PRs (because Prettier will format the code and will add new lines in it). So, it's a one-time cause but will help us in long run. We can add more tests in the future, to check the PR implementation in the right way.

• Do you want to work on this issue? Yes sure, I want to work on this issue. But first, I will be looking for approvals from maintainers, founders and community managers.

I will like to invite @fmvilas @magicmatatjahu @derberg @alequetzalli for discussion and approval on this issue. Since this is going to be a major change in codebase, I will like to inform community members regarding this. Secondly, will it be okay if we apply formatter (Prettier) on docs and blogs to format it? @alequetzalli @derberg

[derberg]

I'm not a fan of pre-commit hooks if they are not really needed. Have a look at https://github.com/asyncapi/cli/issues/198

I definitely think we should use eslint and best if we apply the same config that we have in other repos to be consistent.

Secondly, will it be okay if we apply formatter (Prettier) on docs and blogs to format it? @alequetzalli @derberg

do you mean prettier can be used with Markdown files? I had no idea 😄 are we sure it won't break anything? in the end it is not pure markdown but mdx 🤔 maybe there is some eslint plugin?

[akshatnema]

I'm not a fan of pre-commit hooks if they are not really needed. Have a look at https://github.com/asyncapi/cli/issues/198

Yeah, I can understand that you have already setup your VS code on auto-linting the code on save. But, this is not a default feature of VS Code or any other code editor. This setup may not be in any other contributor's editor. Therefore, I preferred to have it as a DevDependencies in project and set a script to run all necessary tests before. Secondly, I have made plan for adding all the tests in pre-commit including eslint fix, prettier, run build and more, if you specified.

do you mean prettier can be used with Markdown files? I had no idea 😄 are we sure it won't break anything? in the end it is not pure markdown but mdx 🤔 maybe there is some eslint plugin?

Yes, Prettier allows mdx code formatting. I have tried it on my local system. I am not sure of eslint plugin but prettier definitely provides formatting on both js and mdx.

[quetzalliwrites]

I'm also not a fan of pre-commit hooks. 🪝

I also don't know that I want to add prettier because linting issues are hard to troubleshoot and I'm concerned that this could hamper contributions 🤔

[imabp]

I guess, you can close this.

[akshatnema]

Fine, I can understand why everyone is reluctant for using husky and prettier. It is my suggestion, but it's totally up to you all. I was just thinking of long-term use in this. Sometimes, while a project becomes open-source, contributors from outside, don't take much care of the code readability and code formatting. Secondly, when a project grows and accepts contributions on large scale, it is better to have tests in the project to have proper working and maintenance of a deployed project. It prevents a commit to be made to the project that makes it produce breaking changes on production.

That's all my logic and perspective was, to make this issue. If you still think that there should be proper linting and its fixes, I can make a PR but want to hear from you regarding this, whether this is needed or not.

[imabp]

Hello @akshatnema

You are right, it does help. But please read this.

I have learned one thing from @fmvilas and @derberg, always do what is required by the community. We can have lot of things, but if its not required by the community, its not worth putting the effort, provided everyone has limited time.

I will suggest you, to think in terms of what's required, take some inputs from the AsyncAPI community, and this will help you to add more quality contributions.

[quetzalliwrites]

I have learned one thing from @fmvilas and @derberg, always do what is required by the community. We can have lot of things, but if its not required by the community, its not worth putting the effort, provided everyone has limited time. I will suggest you, to think in terms of what's required, take some inputs from the AsyncAPI community, and this will help you to add more quality contributions.

Yes! I feel that this explains it so well.

And I totally feel you because I too would love to make 1 million changes to the docs, but I have to acknowledge that not all my ideas make sense from an open source community perspective. 😄👍🏽👍🏽

[akshatnema]

Yepp, I understood well. Thanks for your guidance and views on the issue @imabp @alequetzalli. I will like to now close the issue as it isn't needed in the project right now. Thanks to all the contributors here for sharing their views and ideas. With this closing comment, I am closing this issue now. Will love to see you soon in the next GitHub discussion 😄.

[derberg]

well we still need eslint 😄 but it can wait until end of March, maybe someone will first work on https://github.com/asyncapi/community/issues/238 during OpenForce 🤞🏼 I think this detail was lost in the discussion about prettier and husky