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

add a cache delete action #4

Merged
merged 1 commit into from
Nov 24, 2024
Merged

Conversation

aayushshah15
Copy link
Contributor

@aayushshah15 aayushshah15 commented Nov 24, 2024

Important

Adds a GitHub Action to delete Blacksmith caches, updates dependencies, and switches CI/CD workflows from yarn to npm.

  • Behavior:
    • Adds GitHub Action in action.yml to delete Blacksmith caches using key and optional version inputs.
    • Implements cache deletion logic in src/main.ts, handling success, cache not found, and errors.
  • Dependencies:
    • Adds @actions/core and node-fetch to dependencies in package.json.
    • Updates devDependencies with @vercel/ncc, @types/node, @types/node-fetch, and typescript.
  • Build Configuration:
    • Replaces rollup with ncc in package.json build script.
  • CI/CD:
    • Replaces yarn with npm in build.yaml workflow.

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

Copy link

@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 97524b0 in 1 minute and 29 seconds

More details
  • Looked at 149 lines of code in 4 files
  • Skipped 4 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. src/main.ts:22
  • Draft comment:
    Ensure BLACKSMITH_CACHE_TOKEN is defined before using it in the Authorization header to avoid sending requests with an empty token.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. rollup.config.js:13
  • Draft comment:
    Ensure that @actions/core is installed as a dependency since it's marked as external and used at runtime.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The rollup.config.js file specifies @actions/core as an external dependency, which is correct since it's a runtime dependency.

Workflow ID: wflow_dZeY5ioMV8fO6GFz


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from 97524b0 to aebdad4 Compare November 24, 2024 19:47
Copy link

@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. Incremental review on aebdad4 in 36 seconds

More details
  • Looked at 150 lines of code in 4 files
  • Skipped 3 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. src/main.ts:22
  • Draft comment:
    Ensure BLACKSMITH_CACHE_TOKEN is set in the environment variables to avoid authorization errors.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_HReECT8GQlkRg1Hz


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from aebdad4 to 02d4638 Compare November 24, 2024 19:49
Copy link

@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. Incremental review on 02d4638 in 32 seconds

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

Workflow ID: wflow_DEbREpIvtXve4C98


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from 02d4638 to e72e5ff Compare November 24, 2024 19:51
Copy link

@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. Incremental review on e72e5ff in 37 seconds

More details
  • Looked at 177 lines of code in 5 files
  • Skipped 3 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/test.yaml:12
  • Draft comment:
    The workflow is using mkdir-action, which is not relevant to the new cache delete action. Update this to use the new action.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_xGolGO8gcwfjTGt9


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from e72e5ff to 20e89c0 Compare November 24, 2024 19:52
Copy link

@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. Incremental review on 20e89c0 in 41 seconds

More details
  • Looked at 210 lines of code in 6 files
  • Skipped 3 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. package.json:21
  • Draft comment:
    The packageManager field still lists yarn, but the workflow files have been updated to use npm. Consider updating this field to reflect the change to npm.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. .github/workflows/test.yaml:10
  • Draft comment:
    The runs-on value is set to blacksmith, which is not a standard GitHub runner. Ensure this is intentional and that the runner is correctly configured in your environment.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The test.yaml workflow file has a runs-on value of blacksmith, which is not a standard GitHub runner. This might be intentional, but it's worth noting in case it's an oversight.

Workflow ID: wflow_XLojdiZqXTfzExjW


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from 20e89c0 to 829272e Compare November 24, 2024 19:54
Copy link

@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. Incremental review on 829272e in 52 seconds

More details
  • Looked at 212 lines of code in 6 files
  • Skipped 3 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/test.yaml:10
  • Draft comment:
    The runs-on: blacksmith might not be available in all environments. Ensure that the runner is properly configured and available.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. package.json:22
  • Draft comment:
    The packageManager is set to yarn@4.5.1, but the workflows use npm. Consider updating for consistency.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_qwHLQt8s6ASSwxlv


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from 829272e to ed6e784 Compare November 24, 2024 19:55
Copy link

@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. Incremental review on ed6e784 in 45 seconds

More details
  • Looked at 213 lines of code in 6 files
  • Skipped 3 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/test.yaml:10
  • Draft comment:
    The runs-on value blacksmith is not a standard GitHub runner. Ensure that this custom runner is correctly set up and available for use.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_oyrVMyQjr6YgX1mN


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from ed6e784 to b016748 Compare November 24, 2024 19:56
Copy link

@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. Incremental review on b016748 in 44 seconds

More details
  • Looked at 215 lines of code in 6 files
  • Skipped 3 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/test.yaml:10
  • Draft comment:
    Ensure that the blacksmith runner is correctly set up and available. If not, consider using a standard runner like ubuntu-latest.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_PCc6DHzxEceqPGFU


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from b016748 to f0ea7bc Compare November 24, 2024 19:58
Copy link

@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 f0ea7bc in 45 seconds

More details
  • Looked at 218 lines of code in 6 files
  • Skipped 3 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. .github/workflows/test.yaml:10
  • Draft comment:
    The 'runs-on' directive has been changed to 'blacksmith', which might not be a standard runner. Ensure this is intentional and that the runner is correctly configured for the environment.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. .github/workflows/build.yaml:21
  • Draft comment:
    Ensure that 'npm ci' is the correct command for installing dependencies in your CI environment, as it assumes a package-lock.json is present and up-to-date.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR description mentions switching from yarn to npm, but the test.yaml file still contains references to 'mkdir-action', which seems unrelated to the cache deletion functionality. This might be an oversight or leftover from previous configurations.
3. package.json:8
  • Draft comment:
    The 'test' script is missing from the package.json file. If testing is part of the CI/CD process, consider adding a test script.
  • 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, which is the removal of the 'test' script. However, the comment is speculative, as it suggests adding the script back only if testing is part of the CI/CD process. This does not indicate a definite issue that needs to be addressed.
    The comment could be seen as informative rather than actionable, as it does not point out a definite problem but rather suggests a potential action based on an assumption.
    The comment does not provide strong evidence of a necessary code change, as it is based on an assumption about the CI/CD process.
    The comment should be deleted because it is speculative and does not point out a definite issue that requires a code change.

Workflow ID: wflow_gLY9jDNeukqu6jp3


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

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch 2 times, most recently from 3805309 to d189f84 Compare November 24, 2024 20:00
Copy link

@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 3805309 in 52 seconds

More details
  • Looked at 227 lines of code in 6 files
  • Skipped 1 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. package.json:8
  • Draft comment:
    The test script is missing in the scripts section. Since the CI/CD workflow uses npm test, consider adding a test script to the package.json.
  • 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 removal of the 'test' script. If the CI/CD workflow indeed relies on 'npm test', then the removal could break the workflow, making the comment valid. However, without more context on the CI/CD setup, it's speculative.
    I might be missing the context of the CI/CD setup, which could mean the 'test' script is no longer needed. The comment assumes a dependency on 'npm test' without evidence.
    The removal of the 'test' script is a change in the diff, and if the CI/CD workflow is known to use 'npm test', the comment is valid. However, without explicit evidence, it's speculative.
    The comment is speculative without evidence of the CI/CD workflow's reliance on 'npm test'. It should be deleted unless there's strong evidence supporting the claim.
2. .github/workflows/test.yaml:12
  • Draft comment:
    The uses: ./mkdir-action step seems unrelated to the current action's purpose of deleting caches. Ensure this step is relevant to the cache deletion functionality.
  • Reason this comment was not posted:
    Comment did not seem useful.
3. .github/workflows/build.yaml:32
  • Draft comment:
    The test script is missing in the scripts section of package.json. Since this workflow uses npm test, consider adding a test script to the package.json.
  • Reason this comment was not posted:
    Marked as duplicate.
4. action.yml:16
  • Draft comment:
    The test script is missing in the scripts section of package.json. Since the CI/CD workflow uses npm test, consider adding a test script to the package.json.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_QqdeBP2nJ4k0Gjsw


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

4 days left in your free trial, upgrade for $20/seat/month or contact us.

Copy link

@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. Incremental review on d189f84 in 43 seconds

More details
  • Looked at 226 lines of code in 6 files
  • Skipped 1 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/test.yaml:13
  • Draft comment:
    Consider setting sparse-checkout-cone-mode to true for more efficient sparse checkouts unless there's a specific reason not to.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The sparse-checkout-cone-mode is set to false in .github/workflows/test.yaml, which might not be necessary. Cone mode is generally more efficient and should be used unless there's a specific reason not to.
2. package.json:17
  • Draft comment:
    Consider removing typescript-eslint from devDependencies if it's not being used.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The package.json file still contains typescript-eslint in devDependencies, but there is no script or configuration indicating its use. It might be unnecessary.

Workflow ID: wflow_PEoCnNxm0KqBug1n


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

logError(err);
process.exitCode = 1;
import { getInput, setFailed, info } from "@actions/core";
import fetch from "node-fetch";
Copy link

Choose a reason for hiding this comment

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

Consider using the native fetch API available in Node.js 18 and later instead of node-fetch.

Suggested change
import fetch from "node-fetch";

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from d189f84 to cb5aa96 Compare November 24, 2024 20:06
Copy link

@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 cb5aa96 in 42 seconds

More details
  • Looked at 230 lines of code in 6 files
  • Skipped 3 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. .github/workflows/test.yaml:10
  • Draft comment:
    The 'runs-on' value 'blacksmith' is not a standard GitHub runner. Ensure this is intended and configured correctly.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. .github/workflows/build.yaml:20
  • Draft comment:
    Ensure that 'npm ci' is the correct command for installing dependencies, as it removes the existing node_modules folder and installs only the dependencies listed in package-lock.json. This is a change from 'yarn', so verify that this behavior is intended.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR description mentions switching from yarn to npm, but the test.yaml file still contains a reference to 'mkdir-action', which seems unrelated to the current action. This might be a leftover from previous configurations and should be updated or removed if not needed.
3. package.json:7
  • Draft comment:
    The 'test' script is missing from the package.json. If testing is required, consider adding a test script.
  • 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 removal of the 'test' script. However, the comment is speculative, suggesting adding a test script if testing is required, which is not a definite issue. The rules specify not to make speculative comments.
    The comment might be useful if testing is a critical part of the project, but without context, it's speculative. The rules advise against speculative comments.
    The rules are clear about not making speculative comments, and without strong evidence that the removal of the 'test' script is an issue, the comment should be deleted.
    The comment is speculative and does not point out a definite issue, so it should be deleted.
4. src/main.ts:23
  • Draft comment:
    Ensure that the 'BLACKSMITH_CACHE_TOKEN' environment variable is set and securely managed, as it is used for authorization.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_vA98Kh2dCjAl67mS


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

4 days left in your free trial, upgrade for $20/seat/month or contact us.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from cb5aa96 to f6f7b62 Compare November 24, 2024 20:08
Copy link

@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. Incremental review on f6f7b62 in 43 seconds

More details
  • Looked at 274 lines of code in 7 files
  • Skipped 4 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/test.yaml:12
  • Draft comment:
    The step using ./mkdir-action seems unrelated to the cache deletion functionality. Consider removing or updating it to reflect the current action's purpose.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.
2. package.json:6
  • Draft comment:
    Ensure the syntax prettier --write --cache . !dist correctly excludes the dist directory. Consider using prettier --write --cache "**/*" "!dist/**" for better compatibility.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The package.json file has a format script that excludes the dist directory, which is a common practice to avoid formatting built files. However, the syntax used might not work as intended with some shell environments.

Workflow ID: wflow_WZVgjfwND8eRwQl0


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

src/main.ts Outdated
method: "DELETE",
headers: {
Accept: "application/json; version=6.0-preview.1",
"X-Github-Repo-Name": process.env["GITHUB_REPO_NAME"] ?? "",
Copy link

Choose a reason for hiding this comment

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

Consider adding a check to ensure BLACKSMITH_CACHE_TOKEN is set before making the request. This will help in providing a clear error message if the token is missing.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from f6f7b62 to 334971f Compare November 24, 2024 20:09
Copy link

@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. Incremental review on 334971f in 50 seconds

More details
  • Looked at 251 lines of code in 7 files
  • Skipped 4 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/test.yaml:10
  • Draft comment:
    Ensure that runs-on: blacksmith is intentional, as blacksmith is not a standard GitHub runner.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The runs-on field in the test workflow has been changed to blacksmith, which is not a standard GitHub runner. This might be intentional, but it's worth noting in case it's an error.
2. package.json:3
  • Draft comment:
    Consider whether removing "type": "module" is intentional, as it affects module resolution and usage in Node.js.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The package.json file has removed the type: "module" field. This might affect how modules are imported and used, especially if ES modules were being used before.

Workflow ID: wflow_MCfFSx5nDHPNR6B5


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

logError(err);
process.exitCode = 1;
import { getInput, setFailed, info } from "@actions/core";
import fetch from "node-fetch";
Copy link

Choose a reason for hiding this comment

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

Consider using the native Fetch API available in Node.js 18 and later instead of node-fetch, as node-fetch is deprecated for these versions.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from 334971f to c1ffa8b Compare November 24, 2024 20:10
Copy link

@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. Incremental review on c1ffa8b in 57 seconds

More details
  • Looked at 270 lines of code in 7 files
  • Skipped 4 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. package.json:6
  • Draft comment:
    The format script uses an invalid exclusion pattern !dist. Consider using a .prettierignore file to exclude the dist directory.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. action.yml:15
  • Draft comment:
    The using field specifies node20, which is not a standard version. Consider using node16 or node18, which are supported by GitHub Actions.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_H5aLYNY8yJiccXdu


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

const baseUrl =
process.env.BLACKSMITH_CACHE_URL ||
(process.env.PETNAME?.includes("staging")
? "https://stagingapi.blacksmith.sh/cache"
Copy link

Choose a reason for hiding this comment

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

Consider checking if the BLACKSMITH_CACHE_TOKEN environment variable is set before making the request. If it's not set, you can fail early with a clear error message.

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from c1ffa8b to 4d5a61b Compare November 24, 2024 20:13
Copy link

@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. Incremental review on 4d5a61b in 45 seconds

More details
  • Looked at 270 lines of code in 7 files
  • Skipped 4 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. package.json:11
  • Draft comment:
    node-fetch is deprecated. Consider removing it if you switch to the native fetch API available in Node.js 18 and later.
  • Reason this comment was not posted:
    Marked as duplicate.
2. package.json:6
  • Draft comment:
    Ensure the exclusion syntax !dist in the format script is correct for your setup.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The package.json file has a format script that excludes the dist directory, which is good. However, ensure that the exclusion syntax is correct for your setup.
3. action.yml:15
  • Draft comment:
    node20 supports the native fetch API, consider using it instead of node-fetch.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_VZVBcHrnbxTDwFQW


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

logError(err);
process.exitCode = 1;
import { getInput, setFailed, info } from "@actions/core";
import fetch from "node-fetch";
Copy link

Choose a reason for hiding this comment

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

node-fetch is deprecated. Consider using the native fetch API available in Node.js 18 and later.

Suggested change
import fetch from "node-fetch";

@aayushshah15 aayushshah15 force-pushed the 11-24-add_a_cache_delete_action branch from 4d5a61b to aeea819 Compare November 24, 2024 20:25
Copy link

@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. Incremental review on aeea819 in 46 seconds

More details
  • Looked at 270 lines of code in 7 files
  • Skipped 4 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. package.json:6
  • Draft comment:
    The format script should exclude the dist directory correctly. Consider using a .prettierignore file to specify ignored paths.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The package.json file has a format script that uses Prettier with a --cache option. However, the !dist pattern is incorrect for excluding the dist directory. The correct way to exclude a directory in Prettier is to use the --ignore-path option with a .prettierignore file.
2. .github/workflows/build.yaml:26
  • Draft comment:
    Consider using git diff --exit-code instead of git diff && git diff-index --quiet --exit-code HEAD for a more reliable check for changes.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    In the build.yaml file, the git diff command is used to check for changes after formatting and building. However, this command will not work as expected if there are untracked files. Using git diff --exit-code is more reliable for checking if there are any changes.

Workflow ID: wflow_Su7KlpntCpKJNGMO


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.

4 days left in your free trial, upgrade for $20/seat/month or contact us.

headers: {
Accept: "application/json; version=6.0-preview.1",
"X-GitHub-Repo-Name": process.env["GITHUB_REPO_NAME"] ?? "",
Authorization: `Bearer ${process.env["BLACKSMITH_CACHE_TOKEN"]}`,
Copy link

Choose a reason for hiding this comment

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

Consider checking if process.env["BLACKSMITH_CACHE_TOKEN"] is set before making the request. If not, set a failed status with a clear error message.

@aayushshah15 aayushshah15 merged commit ae29f5d into main Nov 24, 2024
1 check failed
@aayushshah15 aayushshah15 deleted the 11-24-add_a_cache_delete_action branch November 24, 2024 20:26
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.

1 participant