Skip to content

enforce code formatting and linting#175

Open
JPadovano1483 wants to merge 2 commits into
skupperproject:mainfrom
JPadovano1483:james-code-formatting
Open

enforce code formatting and linting#175
JPadovano1483 wants to merge 2 commits into
skupperproject:mainfrom
JPadovano1483:james-code-formatting

Conversation

@JPadovano1483

Copy link
Copy Markdown
Contributor

Overview

Introduces Prettier for code formatting, ESLint for code quality, and Husky to run Prettier and ESLint as a pre-commit hook. The intent is to enforce coding standards in the repo to create uniform styling and enhance code quality.

Setup

  1. Install the following three extensions:
    1. Prettier - esbenp.prettier-vscode
    2. ESLint - dbaeumer.vscode-eslint
    3. EditorConfig - EditorConfig.EditorConfig
  2. Run pnpm install from root
  3. To run Prettier formatter, run pnpm format (`pnpm format:check to see what files WOULD be changed)
  4. To run ESLint's linter, run pnpm lint:fix (pnpm lint to see what errors exist without fixing them)

Husky takes care of running these two scripts as a pre-commit hook, blocking the commit and reporting the problems if ESLint throws errors. ESLint will automatically fix any linting issues (in staged files) that have auto-fixes for them (typically small things like making a variable a const if it is never reassigned). It will then report what the other issues are, and block the commit. If the issues are deemed as acceptable, the hook can be bypassed with git commit -n, though this should be done sparingly.

Some of the values here were set by looking at what the convention seems to be in the controller code (things like 4 space tab width and semicolon usage). Other values were chosen somewhat arbitrarily, like max line length or arrow function parentheses. If you have any strong preferences, let me know and I'll change those.

Leaving this as a draft until an initial pass has been done to fix linting errors (this will block almost every commit until these errors are addressed.

Fixes #170

@JPadovano1483
JPadovano1483 force-pushed the james-code-formatting branch from 5e1a797 to f7bc3ec Compare July 20, 2026 15:32

@fgiorgetti fgiorgetti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works, but it left me a bit in the dark regarding the changes made.
I forced non-conforming changes (incorrect indentation size), introduced a variable with let, and during the commit, it corrected the formatting and also changed let to const (which is actually correct), but I can't see exactly which files were changed.
I'm not sure how complex it would be to show at least a summary of the changes made.

@JPadovano1483

Copy link
Copy Markdown
Contributor Author

It works, but it left me a bit in the dark regarding the changes made. I forced non-conforming changes (incorrect indentation size), introduced a variable with let, and during the commit, it corrected the formatting and also changed let to const (which is actually correct), but I can't see exactly which files were changed. I'm not sure how complex it would be to show at least a summary of the changes made.

That's a good point. The two fixes that come to mind for this would be to:

  1. run only format and lint checking (no auto fixes) during the pre-commit hook. If any changes need to be made, it will fail the commit and show what is erroring in your terminal (without making any changes). Then you could manually run pnpm lint:fix and pnpm format before committing to see what changed.
  2. I think I could add to the lint-staged script to make it capture changes before and after the hook makes changes and then git diff them to show what it changed.

I'll look into this some more.

@JPadovano1483

Copy link
Copy Markdown
Contributor Author

@fgiorgetti I looked a bit deeper into this and was surprised that there isn't really any good support built into these tools for this. I think the two options I mentioned above are probably still the best bet if we wanted to address this. I wrote up an alternative pre-commit script here that shows a diff containing only the changes eslint automatically made during the commit. I ran the diff before running Prettier because that could get very noisy if you say, wrote an entire file with the wrong tab spacing. It would print out that whole file in the diff. The script could also be simplified to show just the files that were modified, depending on how much information you're looking to see.

Let me know if you have any thoughts on a preferred approach here.

@JPadovano1483
JPadovano1483 marked this pull request as ready for review July 22, 2026 20:19
@fgiorgetti

Copy link
Copy Markdown
Member

@fgiorgetti I looked a bit deeper into this and was surprised that there isn't really any good support built into these tools for this. I think the two options I mentioned above are probably still the best bet if we wanted to address this. I wrote up an alternative pre-commit script here that shows a diff containing only the changes eslint automatically made during the commit. I ran the diff before running Prettier because that could get very noisy if you say, wrote an entire file with the wrong tab spacing. It would print out that whole file in the diff. The script could also be simplified to show just the files that were modified, depending on how much information you're looking to see.

Let me know if you have any thoughts on a preferred approach here.

If fixes could be left unstaged, it would solve it.

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.

enforce code formatting and linting

2 participants