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

feat: Add lint and prettier check in workflow #955

Merged
merged 39 commits into from
Dec 9, 2024
Merged

Conversation

plxity
Copy link
Contributor

@plxity plxity commented Dec 5, 2024

Important

Add GitHub Actions workflow for linting and formatting JavaScript and TypeScript files using ESLint and Prettier, with configuration updates and file cleanup.

  • GitHub Actions:
    • Adds lint_js.yml workflow to run ESLint and Prettier on .js and .ts files on push and pull request events.
  • ESLint Configuration:
    • Adds eslint.config.mjs with rules for TypeScript and JavaScript linting.
  • Prettier Configuration:
    • Adds .prettierrc with formatting rules.
  • Package.json:
    • Updates scripts to include eslint, prettier, and prettier:check.
    • Adds dependencies for ESLint and Prettier.
  • File Removal:
    • Removes js/src/cli/add.ts as it is no longer needed.
  • Miscellaneous:
    • Minor formatting changes in several files for consistency.

This description was created by Ellipsis for 1ac2fd1. It will automatically update as commits are pushed.

Copy link

vercel bot commented Dec 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
composio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 9, 2024 6:52pm

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 1d3fe0e in 19 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/lint_js.yml:34
  • Draft comment:
    continue-on-error: false is unnecessary as it is the default behavior. Consider removing it for cleaner code. This applies to line 38 as well.
  • Reason this comment was not posted:
    Confidence changes required: 20%
    The workflow is well-structured, but there's a minor inconsistency in the use of continue-on-error.

Workflow ID: wflow_a3HzDM34N2NgmT42


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 2c000e5 in 1 minute and 11 seconds

More details
  • Looked at 4679 lines of code in 36 files
  • Skipped 2 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. js/package.json:16
  • Draft comment:
    Consider removing the --max-warnings=177 option from the ESLint script to ensure all warnings are addressed rather than ignored.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The ESLint command in the package.json file includes a specific number of maximum warnings (177). This seems arbitrary and might not be suitable for all cases. It's better to address warnings rather than setting a high threshold.
2. js/src/sdk/utils.ts:29
  • Draft comment:
    The function name converReqParamForActionExecution seems to have a typo. Consider renaming it to convertReqParamForActionExecution.
  • Reason this comment was not posted:
    Comment was on unchanged code.
3. js/src/sdk/base.toolset.ts:13
  • Draft comment:
    The function converReqParamForActionExecution is used here. Ensure the function name is corrected to convertReqParamForActionExecution to match the intended naming convention. This applies to other instances in this file as well.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_sGvrTMxbdgjn0BZ3


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

import tseslint from "typescript-eslint";


/** @type {import('eslint').Linter.Config[]} */
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider specifying a parser in the ESLint configuration, such as @typescript-eslint/parser, to ensure proper linting of TypeScript files.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 21d3f78 in 31 seconds

More details
  • Looked at 264 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. js/src/cli/index.ts:8
  • Draft comment:
    The import statement for './add' is unnecessary since 'add.ts' was removed. Please remove this import.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_35cwVgwV4sWBFJVt


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 91668f2 in 17 seconds

More details
  • Looked at 17 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/lint_js.yml:38
  • Draft comment:
    Add a newline at the end of the file for better compatibility with version control systems.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The YAML file lacks a newline at the end, which is a best practice for version control systems.

Workflow ID: wflow_Fd7dq9infTs0LrnG


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

github-actions bot commented Dec 5, 2024

This comment was generated by github-actions[bot]!

JS SDK Coverage Report

📊 Coverage report for JS SDK can be found at the following URL:
https://pub-92e668239ab84bfd80ee07d61e9d2f40.r2.dev/coverage-12242300096/coverage/index.html

📁 Test report folder can be found at the following URL:
https://pub-92e668239ab84bfd80ee07d61e9d2f40.r2.dev/html-report-12242300096/html-report/report.html

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on b016720 in 20 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/lint_js.yml:33
  • Draft comment:
    Ensure that the package.json scripts section has an eslint script defined. If it was previously lint, update it to eslint to match this change.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_7IlgrYxEl0qRzWs9


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 5c9135d in 19 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. js/package.json:16
  • Draft comment:
    Consider reviewing the increase in --max-warnings from 177 to 193. Ensure this change is intentional and not overlooking new warnings.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change in max-warnings from 177 to 193 in the eslint script might indicate an increase in allowed warnings, which could be a temporary measure. It's important to ensure that this is intentional and not a result of overlooking new warnings.

Workflow ID: wflow_m28KFcoE6vrpjnBv


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 6345851 in 19 seconds

More details
  • Looked at 38 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/lint_js.yml:34
  • Draft comment:
    Add a newline at the end of the file for better compatibility with Unix-based systems.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The workflow file is missing a newline at the end, which is a common best practice for text files.

Workflow ID: wflow_odKwHDXcboro7YuJ


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 5c21131 in 37 seconds

More details
  • Looked at 43 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/lint_js.yml:28
  • Draft comment:
    The node version '22' is not valid. Consider using 'lts/*' to ensure compatibility with the latest LTS version.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is about a change made in the diff, specifically the node version. The suggestion to use 'lts/*' is a valid point for ensuring compatibility with the latest LTS version. However, the comment states '22' is not valid, which might not be accurate if '22' is a valid version. The comment is not speculative and directly addresses a change made in the diff.
    I might be missing the context of whether '22' is a valid Node.js version. If '22' is indeed a valid version, the comment's claim of it being invalid is incorrect.
    I should verify if '22' is a valid Node.js version. If it is, the comment should be deleted as it incorrectly states that '22' is not valid.
    Check if '22' is a valid Node.js version. If it is, delete the comment as it incorrectly states that '22' is not valid.

Workflow ID: wflow_KPbS2Tk5lDB3pCiT


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 633f47f in 1 minute and 6 seconds

More details
  • Looked at 7804 lines of code in 43 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. js/src/sdk/utils/errors/src/formatter.ts:81
  • Draft comment:
    The default case in the getAPIErrorDetails function is redundant as it performs the same operations as the specific cases above it. Consider refactoring to avoid redundancy.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code in getAPIErrorDetails function is handling different error codes and returning appropriate error details. However, the default case is redundant because it is essentially doing the same thing as the specific cases above it. This can be refactored to avoid redundancy.

Workflow ID: wflow_CdoLkHfSZfRl8XEQ


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 06f7888 in 1 minute and 19 seconds

More details
  • Looked at 101 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. js/src/cli/apps.ts:85
  • Draft comment:
    Consider rethrowing the error after logging it to avoid silent failures.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. js/src/cli/apps.ts:108
  • Draft comment:
    Consider rethrowing the error after logging it to avoid silent failures.
  • Reason this comment was not posted:
    Marked as duplicate.
3. js/src/sdk/models/connectedAccounts.ts:64
  • Draft comment:
    Consider renaming redirectUri to redirectUrl in the constructor for consistency.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_qANAfSkZtRoL08nl


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on d7e7e0f in 1 minute and 23 seconds

More details
  • Looked at 68 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. js/src/sdk/utils/pusher.ts:123
  • Draft comment:
    Consider using const instead of var for variable declaration to ensure block scoping and prevent potential hoisting issues.
    const channel = PusherUtils.pusherClient.subscribe(
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. js/src/sdk/models/Entity.spec.ts:8
  • Draft comment:
    Consider using const instead of var for variable declaration to ensure block scoping and prevent potential hoisting issues. This issue is also present in workspace.ts.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_dqo1TcskLouxpC1Q


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 1ac2fd1 in 1 minute and 1 seconds

More details
  • Looked at 60 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_5OBKHR3XFIvwPr7b


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@himanshu-dixit himanshu-dixit enabled auto-merge (squash) December 9, 2024 18:58
@himanshu-dixit himanshu-dixit merged commit 7dce021 into master Dec 9, 2024
14 checks passed
@himanshu-dixit himanshu-dixit deleted the ENG-2785 branch December 9, 2024 19:06
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