126: Add linter command to CI

[JensAstrup]

No description provided.

[github-actions]

Given the changes in the .github/workflows/lint.yml file, there are a couple of points to note:

1. The Node.js version specified under the "Use Node.js 18.x" step is inconsistent with its description. The node-version is set to 21.x, which conflicts with the name of the step indicating it should use Node.js 18.x. This should be corrected to ensure clarity and correct version usage.

2. Everything else in the proposed workflow looks well-structured and adheres to best practices for setting up a GitHub Actions workflow for linting. The usage of event triggers (push to develop branch and specific pull_request types) alongside the specified paths (src/** and tests/**) ensures that linting is performed efficiently and only when necessary.

3. The step descriptions are clear and the sequential steps from checking out the code to installing dependencies and finally running the linter are logically ordered.

To summarize, the only required change is to ensure the version of Node.js specified is consistent with the step name or update the step name to accurately reflect the version being used. Other than that, the proposed GitHub Actions workflow is well-configured to maintain code quality through linting.

[github-actions]

The changes in the .github/workflows/lint.yml and src/client.ts files look good with only minor concerns regarding best practices and consistency:

1. .github/workflows/lint.yml file:

   • The node-version specified in the "Use Node.js 18.x" step is 21.x, which doesn't match the step's name. This might be an oversight. If the goal is actually to use Node.js version 18.x, then this needs to be corrected to "node-version: '18.x'". It's essential for maintaining consistency and avoiding confusion about which version of Node.js the workflow is intended to use.

2. src/client.ts file:

   • The addition of the curly brace on a new line before the else if construct (+ } + else if (!process.env.SHORTCUT_API_KEY) throw new Error('Shortcut API Key not found')) introduces a style inconsistency with common TypeScript/JavaScript practices. Typically, the } else if { is used on the same line to maintain readability and adhere to standard styling conventions.

Make sure to address these minor issues to ensure the code is clean, consistent, and follows the best practices.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.48%. Comparing base (6bc25f0) to head (da64156).

Additional details and impacted files

@@            Coverage Diff            @@
##           develop     #136    +/-   ##
=========================================
  Coverage    91.48%   91.48%            
=========================================
  Files           42       42            
  Lines         1785     1786     +1     
  Branches       100      208   +108     
=========================================
+ Hits          1633     1634     +1     
  Misses         152      152            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Reviewing the changes from your recent commit, here are some observations and suggestions:

1. Regarding the .github/workflows/lint.yml addition:

   • The syntax and steps defined for the GitHub action to run linters on push and pull requests targeting develop branch, focusing on src/** and tests/** paths, are properly set. This strategy will help in maintaining code quality consistently across the project.

2. In the package.json modifications:

   • You updated the lint script to specifically target the src and tests directories for ESLint. This aligns well with the paths defined in the GitHub action and ensures that linting is focused on the relevant parts of the project.

3. Changes to the src/base-resource.ts, src/client.ts, and similar structure modifications in other files:

   • It's noticed that you've adjusted the else blocks to be on a new line after the closing brace of the preceding block (}) instead of on the same line. This change is not consistent with common JavaScript/TypeScript styling practices. Typically, the else statement follows the closing brace of the if block on the same line. Ensuring consistency in coding style across the project is crucial for readability and maintainability. Consider aligning with the prevalent style guides or the existing project coding standards.

4. Regarding the src/stories/history/actions/history-action.ts changes:

   • The spacing and organization of the code within HistoryAction class have improved, making the separation between methods clearer which enhances readability. However, the else block statement styling mentioned earlier also applies here.

5. For the src/uploaded-files/uploaded-files-service.ts adjustments:

   • The separation of the catch block onto a new line, similar to the else blocks mentioned before, needs reconsideration for consistency with common coding styles.

6. In general:

   • It would be beneficial to integrate a formatter like Prettier or enforce a linter rule regarding the positioning of else and catch blocks to ensure uniformity across the codebase.

Your initiative to improve the project's maintainability and readability through these changes is commendable. Paying attention to maintaining consistent coding styles and following best practices can further enhance the quality of the changes.