apache/answer

Add linting step to make ui for consistent code quality

Opened this issue · 5 comments

Problem

While building the UI using the make ui command, the following sequence of commands is run:

@cd ui && pnpm pre-install && pnpm build && cd -

During development, we ran into an issue where linting was not done consistently. Specifically, I ran npm run lint, but other developers didn't, which made it harder to find and fix the issue, especially with Issue #1217.

Suggested Solution

To improve consistency and avoid future confusion, we should add the npm run lint step to the make ui process. This will ensure linting is always run as part of the build process.

Proposed Update:

ui:
    @cd ui && pnpm pre-install && pnpm build && npm run lint && cd -

This ensures that linting is always performed after building the UI, maintaining consistent code quality across all developers.

Alternatives Considered

  • CI/CD Pipeline Linting: Ensure linting is mandatory in the CI/CD pipeline to catch any linting issues before deployment.
  • Pre-commit Hooks: Use pre-commit hooks (e.g., Husky) to enforce linting before every commit, ensuring developers don’t forget to lint locally.

Impact

Adding this change will:

  • Ensure consistent linting across all developers.
  • Prevent hard-to-find bugs related to inconsistent code quality.
  • Make the development process smoother and more predictable.

I want to solve this issue.

It is meaningless to add lint in the build phase, because the build product will be compressed, and it is meaningful in the development and submission phase.

At the beginning, we ran lint through husky's pre-commit phase during the submission phase, so as to ensure the agreed code style. Later, the verification was canceled due to some other reasons.

@shuashuai Thanks for the feedback! I understand that linting in the build phase may not be as useful since the build product is compressed.

However, adding linting to the build process ensures it's always run, even if developers forget to do it manually. I agree that Husky pre-commit linting is a good approach. Another idea could be to apply linting in the npm run start command, so it's automatically run during development without relying on developers to remember it.

What do you think?

@Mindslayer001

You can first withdraw this solution from the previous PR and submit a separate PR, so that it will not affect the submission of the previous PR.

I need to think again about how to add lint verification scheme when submitting code, because the method you mentioned above is not a standard solution, or see if other people in the community have better methods.