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

chore: force push npm package #1245

Open
wants to merge 9 commits into
base: canary
Choose a base branch
from
Open

chore: force push npm package #1245

wants to merge 9 commits into from

Conversation

sxlijin
Copy link
Collaborator

@sxlijin sxlijin commented Dec 13, 2024

Important

Modifies GitHub Actions to focus on npm publishing and updates repository URL in package.json.

  • Workflow Changes:
    • Adds branch sam/npm-token-fix to trigger workflow in release.yml.
    • Comments out jobs for Python, Ruby, GitHub, Open VSX, and VSCode Marketplace publishing in release.yml.
  • NPM Publishing:
    • Modifies publish-to-npm job to run independently without all-builds dependency in release.yml.
    • Adds run-id and github-token parameters to actions/download-artifact step in publish-to-npm.
  • Package.json Update:
    • Corrects repository URL format in package.json.

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

Copy link

vercel bot commented Dec 13, 2024

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

Name Status Preview Comments Updated (UTC)
baml ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 13, 2024 8:45pm

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 b3aca84 in 36 seconds

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

Workflow ID: wflow_7J2fSnCJBmK8WY0i


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.

@@ -117,6 +119,8 @@ jobs:
with:
pattern: bindings-*
path: engine/language_client_typescript/artifacts
run-id: 12318760112
Copy link
Contributor

Choose a reason for hiding this comment

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

The 'run-id' and 'github-token' fields in the 'actions/download-artifact' step are not standard and might cause issues. They should be removed unless they are part of a custom action that requires them.

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 0fba3a3 in 9 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. engine/language_client_typescript/package.json:7
  • Draft comment:
    The change from 'https' to 'git+' in the repository URL is unnecessary for npm package.json files. Consider reverting to 'https://github.com/BoundaryML/baml.git'.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change from 'https' to 'git+' in the repository URL is unnecessary for npm package.json files. It doesn't affect the functionality but could be misleading.

Workflow ID: wflow_R5a1u2NWBJXsjKyH


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. Incremental review on 53656cb in 32 seconds

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

Workflow ID: wflow_HMVjM2QoFvjwrp50


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.

@@ -128,99 +132,100 @@ jobs:

- name: Publish
run: |
npm adduser
Copy link
Contributor

Choose a reason for hiding this comment

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

The npm adduser command is unnecessary here as authentication is managed via the NODE_AUTH_TOKEN. Consider removing it to streamline the workflow.

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 9f25456 in 14 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/release.yml:113
  • Draft comment:
    Ensure that the registry-url setting does not conflict with other registry settings in the workflow.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The addition of registry-url is correct and aligns with the intent to focus on npm publishing. However, it's important to ensure that this change does not conflict with any other registry settings in the workflow.

Workflow ID: wflow_VJejVgbiPgZyRBDf


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 32266aa in 16 seconds

More details
  • Looked at 25 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/release.yml:111
  • Draft comment:
    The comment is incorrect. The NODE_AUTH_TOKEN is used as an environment variable, as specified in the env section. Update the comment to reflect this.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_wZuXBqYFLf5847r6


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 961d18b in 28 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/release.yml:114
  • Draft comment:
    Consider uncommenting the registry-url line to ensure the npm registry is explicitly set to https://registry.npmjs.org. This can prevent issues if the default registry is not npmjs.org.
  • 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 commenting out of the registry-url line. The suggestion to uncomment it is based on the potential issue of the default registry not being npmjs.org. However, the comment is speculative as it assumes a problem that may not exist. The change might have been intentional, and the comment does not provide strong evidence that the change is incorrect.
    The comment could be seen as a valid suggestion to prevent potential issues, but it lacks strong evidence that the change is incorrect or problematic. It is speculative and assumes a problem without certainty.
    While the comment is speculative, it does address a potential issue that could arise from the change. However, without strong evidence that the change is incorrect, the comment does not meet the criteria for being kept.
    The comment should be deleted as it is speculative and lacks strong evidence that the change is incorrect or problematic.

Workflow ID: wflow_3hQpb95xCq3FRWZ8


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 033abba in 9 seconds

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

Workflow ID: wflow_LceDPPoUQDSQ2rbR


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

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