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

Tool Installation: Prettier #20

Open
wants to merge 15 commits into
base: f24
Choose a base branch
from
Open

Tool Installation: Prettier #20

wants to merge 15 commits into from

Conversation

emshyu
Copy link

@emshyu emshyu commented Oct 23, 2024

Prettier is a static analysis tool that checks for consistent code style and has code reformatting capabilities. The tool will need further testing of rules so that it can coexist with EsLint. Currently, both tools can be run separately. Attached below is the command line stdout and stderr output of a successful run of 'npx prettier . --check' on our current nodebb codebase, a command that checks the style of the code.

Proof of installation:

  • .prettierignore file
  • .prettierrc file

Proof of running the tool successfully:
prettierinstallationproof.txt

KEY DETAILS FOR COMPLETE INTEGRATION:

  • I added installations of Prettier, eslint-config-prettier, a call to prettier that reformats the code to fit standards, and a prettier formatting check to the integration pipeline.
  • Currently, the pipeline passes for the Prettier and Node tests. Because Prettier is a linter, some EsLint tests still fail.
  • To allow Eslint to coexist with Prettier, the workflow installs eslint-config-prettier and .eslintrc extends prettier. However, some rules still fail with EsLint, notably comma-dangle. I attempted to make an eslint.config.js file that can ignore certain rules, however I found challenges getting it to extend nodebb.
  • Prettier ignores all files under the install/, build/, and test/ directories. Reformatting the test cases caused some formatting-specific Node tests to fail, meaning ignoring certain directories when 'npx prettier . --write' (the reformatter) is called allows for complete integration with Node tests.

@coveralls
Copy link

coveralls commented Oct 23, 2024

Pull Request Test Coverage Report for Build 11511413906

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 82.671%

Totals Coverage Status
Change from base Build 11446603378: 0.0%
Covered Lines: 22323
Relevant Lines: 25582

💛 - Coveralls

@emshyu emshyu requested a review from tinchil November 1, 2024 03:23
@emshyu
Copy link
Author

emshyu commented Nov 1, 2024

KEY DETAILS FOR COMPLETE INTEGRATION:

  • I added installations of Prettier, eslint-config-prettier, a call to prettier that reformats the code to fit standards, and a prettier formatting check to the integration pipeline.
  • Currently, the pipeline passes for the Prettier and Node tests. Because Prettier is a linter, some EsLint tests still fail after prettier write is called.
  • To allow Eslint to coexist with Prettier, the workflow installs eslint-config-prettier and .eslintrc extends prettier. However, some rules still fail with EsLint, notably comma-dangle. I attempted to make an eslint.config.js file that can ignore certain rules, however I found challenges getting it to extend nodebb.
  • Prettier ignores all files under the install/, build/, and test/ directories. Reformatting the test cases caused some formatting-specific Node tests to fail, meaning ignoring certain directories when 'npx prettier . --write' (the reformatter) is called allows for complete integration with Node tests.

@emshyu
Copy link
Author

emshyu commented Nov 1, 2024

Because Prettier conflicts with Eslint, despite interventions, this branch will not be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants