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

Testing VSCode release #1060

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

Testing VSCode release #1060

wants to merge 7 commits into from

Conversation

hellovai
Copy link
Contributor

@hellovai hellovai commented Oct 18, 2024

Important

Adds a new workflow for VSCode extension release, modifies existing workflows, and updates package details.

  • New Workflow:
    • Adds build-vscode-release.reusable.yaml for building VSCode extension on vscode-release branch.
    • Includes steps for setting up Rust and Node.js environments, building the extension, and uploading artifacts.
  • Modifications:
    • Removes build-wasm job from release.yml and replaces it with build-vscode-release.
    • Removes push triggers for specific branches in build-python-release.reusable.yaml and build-typescript-release.reusable.yaml.
    • Comments out environment variable setup in build-python-release.reusable.yaml.
  • Misc:
    • Minor whitespace cleanup in build-python-release.reusable.yaml and build-typescript-release.reusable.yaml.
    • Updates package.json in typescript/vscode-ext/packages to change name to Gloo and publisher to Gloo.

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

Copy link

vercel bot commented Oct 18, 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 Oct 18, 2024 10:29pm

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! Reviewed everything up to b08d36c in 31 seconds

More details
  • Looked at 261 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/build-vscode-release.reusable.yaml:63
  • Draft comment:
    Duplicate step for uploading VSCode Extension Artifact. Consider removing one of them to avoid redundancy.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is pointing out a potential issue with duplicate step names, which could lead to confusion or errors in the workflow. The steps have different paths and artifact names, so they are not functionally identical, but the naming could be misleading. This is a valid concern that could require a code change to improve clarity.
    The steps are not functionally redundant since they upload different artifacts. The comment could be misleading if interpreted as suggesting the steps are identical in function.
    While the steps are not functionally redundant, the naming could still cause confusion. Renaming one of the steps for clarity would be a reasonable action to take.
    The comment should be kept because it highlights a potential source of confusion due to duplicate step names, even though the steps are not functionally redundant.
2. .github/workflows/release.yml:41
  • Draft comment:
    The job build-vscode-release is correctly using the new reusable workflow. Ensure that all necessary steps from the old build-wasm job are included in the new workflow if they are still needed.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new reusable workflow for building VSCode releases and removes the existing build logic from the main release workflow. This is a good practice for modularity and reusability. However, there are some redundant steps in the new workflow that can be optimized.

Workflow ID: wflow_LzfSv814IWZg74nf


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 39036f0 in 13 seconds

More details
  • Looked at 85 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/build-vscode-release.reusable.yaml:59
  • Draft comment:
    The artifact name baml-vscode.vsix should be updated to reflect the new package name Gloo for consistency.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_kPcPIacul9xCKVBR


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 d1be482 in 15 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/build-vscode-release.reusable.yaml:59
  • Draft comment:
    Ensure the artifact name and pattern match exactly to avoid download issues. Consider using name: vscode-playground instead of pattern: vscode-playground.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_cA5HE7ieMbtGtwP2


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 962033a in 31 seconds

More details
  • Looked at 57 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. typescript/vscode-ext/packages/package.json:2
  • Draft comment:
    The name field is set to gloo, but the outputs in turbo.json refer to baml-*.vsix. Ensure consistency between these files.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_C3pJoBaEtztqh7pZ


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.

@@ -52,7 +52,7 @@
"dependsOn": ["@gloo-ai/baml-language-server#build"],
"outputs": ["out/**"]
},
"baml#vscode:package": {
"gloo#vscode:package": {
"dependsOn": ["baml-client#build"],
"outputs": ["baml-*.vsix"]
Copy link
Contributor

Choose a reason for hiding this comment

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

The output pattern baml-*.vsix does not match the package name gloo in package.json. Consider updating the output pattern to gloo-*.vsix to maintain consistency.

Suggested change
"outputs": ["baml-*.vsix"]
"outputs": ["gloo-*.vsix"]

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 39cd98f in 17 seconds

More details
  • Looked at 22 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/build-vscode-release.reusable.yaml:49
  • Draft comment:
    Duplicate artifact upload steps. Consider removing one of the Upload VSCode Extension Artifact steps to avoid redundancy.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_SYLDRBQCyOlbtLgA


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 7ab8b02 in 27 seconds

More details
  • Looked at 25 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/build-vscode-release.reusable.yaml:59
  • Draft comment:
    The artifact path uses 'gloo2' but the package name was changed to 'gloo2'. Ensure consistency between the package name and artifact path.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_3bUT2gKbsFoTdk8U


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 d47b8c6 in 13 seconds

More details
  • Looked at 22 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/build-vscode-release.reusable.yaml:16
  • Draft comment:
    Using 'self-hosted' runners requires ensuring the environment is properly configured and maintained. Consider documenting the setup requirements or justifying the change from 'ubuntu-latest'.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of 'self-hosted' runners instead of 'ubuntu-latest' can lead to potential issues if the self-hosted environment is not properly configured or maintained. This change should be justified and documented.

Workflow ID: wflow_yz3bR2O2yb2UlbEL


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