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

QA/Lint: Prettier + ESLint for the webapp #29

Merged
merged 10 commits into from
Aug 6, 2024

Conversation

GODrums
Copy link
Collaborator

@GODrums GODrums commented Aug 3, 2024

Description

Summary

Adds Prettier + ESLint to the webapp. Configs are similar to the ones used in the Artemis-Repo. Triggers are set to:

  • Github Action
    • On Pull Request in /webapp
    • On Push in the develop-branch with changes in /webapp
  • Commands
    • ESLint: npm run lint and npm run lint:fix
    • Prettier: npm run prettier:check and npm run prettier:write

This PR does NOT include Prettier formatting / ESL linting fixes yet, only the initial setup to avoid potential merge conflicts.

Motivation

The project should follow consistent code conventions and standards.
Prettier and ESLint are the most common combination in attaining such goals.

Key Changes

  • Simple / Loose Prettier config similar to .editorconfig
  • ESLint config taken from the Artemis-repo with small adjustments for our tech stack
  • Github workflow for automatic execution of both commands for pull requests (untested for now)

Checklist

  • Review Prettier config options
  • Review ESLint rules
  • Execute workflow at least once for testing

TODOs / Next up

  • Fix existing Prettier formatting errors
  • Fix existing ESLint errors
  • Add commands to Readme / Contributing Guidelines

General

  • PR title is clear and descriptive
  • PR description explains the purpose and changes
  • Code follows project coding standards
  • Self-review of the code has been done
  • Changes have been tested locally
  • Screenshots have been attached (if applicable)
  • Documentation has been updated (if applicable)

@GODrums GODrums added enhancement New feature or request client needs test PR needs to be updated with tests priority:high Crucial tasks needing prompt attention. labels Aug 3, 2024
@GODrums GODrums requested a review from FelixTJDietrich August 3, 2024 23:25
@GODrums GODrums self-assigned this Aug 3, 2024
Copy link
Collaborator

@FelixTJDietrich FelixTJDietrich left a comment

Choose a reason for hiding this comment

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

Thanks for setting it up I have some minor change requests.

webapp/.prettierignore Outdated Show resolved Hide resolved
.github/workflows/webapp-qa.yml Outdated Show resolved Hide resolved
webapp/eslint.config.js Outdated Show resolved Hide resolved
webapp/eslint.config.js Outdated Show resolved Hide resolved
webapp/eslint.config.js Show resolved Hide resolved
webapp/eslint.config.js Outdated Show resolved Hide resolved
@FelixTJDietrich FelixTJDietrich added this to the Initial Project Setup milestone Aug 4, 2024
Copy link
Collaborator

@FelixTJDietrich FelixTJDietrich left a comment

Choose a reason for hiding this comment

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

I removed some of the linting rules and added some new ones, I think you just copied them from Artemis. We can start with the recommended linting rules and then adjust if necessary.

@FelixTJDietrich FelixTJDietrich merged commit 7fec3c2 into develop Aug 6, 2024
1 of 2 checks passed
@FelixTJDietrich FelixTJDietrich deleted the feature/prettier-eslint branch August 6, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client enhancement New feature or request needs test PR needs to be updated with tests priority:high Crucial tasks needing prompt attention.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants