Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CI to lint the codebase #78

Merged
merged 9 commits into from
Jul 3, 2024
Merged

Conversation

azzamsa
Copy link
Contributor

@azzamsa azzamsa commented Jun 27, 2024

Hi, there.

I've found some inconsistencies in the code. So, perhaps this PR will solve the issue.

This PR adds:

The mentioned tools are also included inside the justfile. So, one is able to check locally just using just fmt or just fmt-check.

The repo had prettier and eslint already, but it seems they were not used in previous commits. So, I have added both to CI and the just recipe too.

I use ci: as the commit prefix here. If these commits should belong under a different category, I'd be happy to change it.

One last step is fixing the error that comes from eslint:

Oops! Something went wrong! :(

ESLint: 8.57.0

Error: Error while loading rule '@typescript-eslint/no-floating-promises': You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser.

@azzamsa
Copy link
Contributor Author

azzamsa commented Jun 27, 2024

I have read the CLA Document and I hereby sign the CLA

@azzamsa azzamsa marked this pull request as ready for review June 27, 2024 03:08
@azzamsa
Copy link
Contributor Author

azzamsa commented Jun 29, 2024

@zicklag if you agree with this PR, I will look for a fix for the ESLint issue. Do you have any suggestions on how to fix the issue?

@zicklag
Copy link
Collaborator

zicklag commented Jun 29, 2024

Yeah, I'm interested in this. I did a quick search and it looks like the issue we're running into is the same as this vuejs/eslint-config-typescript#52.

I'm not exactly sure what the fix is, but we may have to either ignore that lint just for CI, or install typescript in CI before running the lint or something.

@azzamsa
Copy link
Contributor Author

azzamsa commented Jul 3, 2024

@zicklag , I believe this is ready to merge. There is a new typo in the latest commits.

I've temporarily disabled ESLint due to the many errors that need fixing. With the release of ESLint 9, we might want to try it to see if it improves our experience.

@zicklag zicklag merged commit 5c68e96 into commune-os:main Jul 3, 2024
2 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 2024
@azzamsa azzamsa deleted the introduce-lint-ci branch July 3, 2024 14:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants