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

build: Pre-release publish vscode extension #26

Merged
merged 10 commits into from
Jul 12, 2024

Conversation

bmingles
Copy link
Collaborator

@bmingles bmingles commented Jul 11, 2024

Extension has been published as a pre-release version to Visual Studio Marketplace
https://marketplace.visualstudio.com/items?itemName=deephaven.vscode-deephaven

We now have the following:

In this PR:

  • Modified .svg logos for targetting 128x128 size without partial pixels
  • Generated 128x128 .png logos for the Marketplace logos
  • Split out developer specific documentation into CONTRIBUTING.md since README contents gets used as the Marketplace

resolves #18

@bmingles bmingles force-pushed the 18-prerelease-vscode-extension branch from da7f46d to ea09fa4 Compare July 11, 2024 21:10
@bmingles bmingles marked this pull request as ready for review July 11, 2024 21:11
@bmingles bmingles requested a review from mofojed July 11, 2024 21:26
Copy link
Member

@mofojed mofojed 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, just a weird bit around the src/jsapi.downloadDhFromServer() part. I think that was just copied/pasted, but may as well fix at the same time.

As for publishing - I'm assuming we'll get deephaven-bot to publish from the GitHub action in time with this ticket, and this is just for manually publishing right now: #28

CONTRIBUTING.md Outdated

The extension dynamically downloads and loads the DH JS API from a running DH Core server.

- `src/jsApi.downloadDhFromServer()`
Copy link
Member

Choose a reason for hiding this comment

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

Typo? Confused about this part. Seems like it was carried over from README.md, must have missed it before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

And yes, #28 will deal with CI deployment presumably from deephaven-bot

@bmingles bmingles requested a review from mofojed July 12, 2024 16:04
CONTRIBUTING.md Outdated
@@ -84,5 +84,5 @@ On subsequent script runs, the session will be re-used and only steps 4 and 5 wi

The extension dynamically downloads and loads the DH JS API from a running DH Core server.

- `src/jsApi.downloadDhFromServer()`
At runtime, `dh-internal.js` and `dh-core.js` are downloaded from the running DH server (default http://localhost:10000). The files are saved to `out/tmp` as `.cjs` modules, and import / export are converted to cjs compatible ones.
- `src/dh/dhc.getDhc()`
Copy link
Member

Choose a reason for hiding this comment

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

I still find this confusing. Why is the function a bullet point? Why is it calling the function? It seems to break up the paragraph unnecessarily, would just remove the bullet point, have the paragraph go "...running DH Core Server. At runtime ...", then at the end maybe have a link "For implementation details, see ./src/dh/dhc.ts#getDhc" or something like that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

@bmingles bmingles requested a review from mofojed July 12, 2024 16:25
@bmingles bmingles merged commit e40799c into main Jul 12, 2024
1 check passed
@bmingles bmingles deleted the 18-prerelease-vscode-extension branch July 12, 2024 17:34
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.

Manual Pre-release deployment to vscode marketplace
2 participants