chore(website): configure eslint config similiar to asyncapi website

jerensl commented 5 months ago

Description

Added eslint automatic fix for modelina-website and added modelina-website to eslint ignore in parent project, so no conflicting rule in the future when we start migrating the same rule used in async website as mentioned here

Related Issue

This PR is being made as mentioned here

netlify[bot] commented 5 months ago

Deploy Preview for modelina ready!

Name	Link
Latest commit	aa619210b01d0db5ae92d0787d06cdab89d58dcd
Latest deploy log	https://app.netlify.com/sites/modelina/deploys/667cc56ed451790008f392bb
Deploy Preview	https://deploy-preview-2031--modelina.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

sonarcloud[bot] commented 5 months ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9448551980

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 92.303%

Totals
Change from base Build 9382165682:	0.0%
Covered Lines:	5996
Relevant Lines:	6327

💛 - Coveralls

jerensl commented 5 months ago

@jerensl, you can also add the eslint and prettier rules from the asyncapi website in this PR and update the PR title accordingly.

@devilkiller-ag After doing some work I figured out this doesn't work because the next cli makes us dirty here, basically next lint did something different with eslint, it doesn't allow us to have multiple configurations for eslint and always counted the root directory project eslint rule as mentioned in this NextJS discussion, this resulted of conflicting rule with the root directory project.

For the solution, I think it would be better to migrate the website as new repositories and use submodules for this repository

@jonaslagoni what is your opinion about this?

jonaslagoni commented 5 months ago

For the solution, I think it would be better to migrate the website as new repositories and use submodules for this repository

At the moment that wont be an option, too complex of a setup for now.

@devilkiller-ag After doing some work I figured out this doesn't work because the next cli makes us dirty here, basically next lint did something different with eslint, it doesn't allow us to have multiple configurations for eslint and always counted the root directory project eslint rule as mentioned in this https://github.com/vercel/next.js/discussions/36440, this resulted of conflicting rule with the root directory project.

It should be possible to create a website-only eslint configuration even through the next cli, if the next cli is even needed to lint the files.

jerensl commented 5 months ago

@devilkiller-ag, I made adjusted to ignore next/lint and use only eslint for modelina-website. The next step is to run the fix command, I tested it on my local device, which changes all over the code in the website and leftover ~100(mostly minor changes) errors that need to be fixed manually

devilkiller-ag commented 5 months ago

@devilkiller-ag, I made adjusted to ignore next/lint and use only eslint for modelina-website. The next step is to run the fix command, I tested it on my local device, which changes all over the code in the website and leftover ~100(mostly minor changes) errors that need to be fixed manually

@jerensl what type of errors are those?

jerensl commented 5 months ago

@devilkiller-ag, I made adjusted to ignore next/lint and use only eslint for modelina-website. The next step is to run the fix command, I tested it on my local device, which changes all over the code in the website and leftover ~100(mostly minor changes) errors that need to be fixed manually

@jerensl what type of errors are those?

Mostly unused variable and hoisting

https://github.com/asyncapi/modelina/assets/54782057/8914cb3c-8970-4629-a431-5460a4ad3580

jerensl commented 5 months ago

Hi @devilkiller-ag if you don't mind I changed the entire website codebase, I made the latest commit for fixing around 100 eslint errors, which left around ~90 warnings related to jsdoc for documentation purposes

devilkiller-ag commented 4 months ago

Hi @devilkiller-ag if you don't mind I changed the entire website codebase, I made the latest commit for fixing around 100 eslint errors, which left around ~90 warnings related to jsdoc for documentation purposes

Okay, No problem!

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9626634076

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 92.304%

Totals
Change from base Build 9611843317:	0.0%
Covered Lines:	5997
Relevant Lines:	6328

💛 - Coveralls

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9627168389

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 92.304%

Totals
Change from base Build 9626689374:	0.0%
Covered Lines:	5997
Relevant Lines:	6328

💛 - Coveralls

jerensl commented 4 months ago

@devilkiller-ag, I want to mention here too, with the deprecating of the majority eslint formatting rule, It's been clear they're not buying into the idea of becoming both formatter and linter simultaneously. So I propose a solution to make separation between both tools which we are using prettier for formatter and eslint as a linter

https://eslint.org/blog/2023/10/deprecating-formatting-rules/

devilkiller-ag commented 4 months ago

@devilkiller-ag, I want to mention here too, with the deprecating of the majority eslint formatting rule, It's been clear they're not buying into the idea of becoming both formatter and linter simultaneously. So I propose a solution to make separation between both tools which we are using prettier for formatter and eslint as a linter

https://eslint.org/blog/2023/10/deprecating-formatting-rules/

Yeah this seems important to me. We should have this separation. Would you like to work on this @jerensl?

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9657860438

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 92.304%

Totals
Change from base Build 9642600004:	0.0%
Covered Lines:	5997
Relevant Lines:	6328

💛 - Coveralls

jerensl commented 4 months ago

Yeah this seems important to me. We should have this separation. Would you like to work on this @jerensl?

Sure

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9689394190

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 92.304%

Totals
Change from base Build 9642600004:	0.0%
Covered Lines:	5997
Relevant Lines:	6328

💛 - Coveralls

devilkiller-ag commented 4 months ago

How's the work going on @jerensl? Is it ready for review?

devilkiller-ag commented 4 months ago

/rtm

devilkiller-ag commented 4 months ago

Thanks @jerensl!

asyncapi-bot commented 4 months ago

:tada: This PR is included in version 3.7.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

asyncapi / modelina