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

refactor: Split out packages #133

Merged
merged 11 commits into from
Sep 20, 2024
Merged

refactor: Split out packages #133

merged 11 commits into from
Sep 20, 2024

Conversation

bmingles
Copy link
Collaborator

@bmingles bmingles commented Sep 13, 2024

  • Moved the jsapi fetching code into a sub npm package
  • Refactored the jsapi fetching code to have zero dependencies on vscode

The new packages directory provides a better silo for temporary code that will eventually become proper npm packages. e.g. enterprise jsapi types will be put in here on subsequent PR. Should also help with early dev of deephaven/deephaven-core#5537

@bmingles bmingles marked this pull request as ready for review September 13, 2024 22:03
@bmingles bmingles requested a review from mofojed September 13, 2024 22:04
@bmingles bmingles force-pushed the 116-2-panel-management branch 3 times, most recently from aed8676 to 2a1dc80 Compare September 18, 2024 16:15
Base automatically changed from 116-2-panel-management to main September 18, 2024 16:20
Copy link

github-actions bot commented Sep 20, 2024

End-to-end Test Summary

Tests 📝Passed ✅Failed ❌Skipped ⏭️Pending ⏳Other ❓Flaky 🍂Duration ⏱️
660000004:38:44
A ctrf plugin

Detailed Test Results

NameStatusmsFlaky 🍂
should default to the correct settingspassed ✅1806
should return custom settings: Empty configspassed ✅250
should return custom settings: Populated configspassed ✅241
should be able to load VSCodepassed ✅922
should only be visible when a supported file type is active: test.groovypassed ✅2634
should only be visible when a supported file type is active: test.pypassed ✅1044
A ctrf plugin

Failed Test Summary

No failed tests ✨

Flaky Test Summary

No flaky tests detected. ✨

@@ -3,14 +3,17 @@ _ignore/**
.vscode/**
.vscode-test/**
src/**
packages/**
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if instead of a vscodeignore we should only be including certain paths? Seems like we're excluding a lot more than we're including, and we need to exclude more stuff everytime we add new things.

Also I'd add a .venv here - I sometimes create a venv when testing this stuff, and then packaging takes forever if I forget I did 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.

Just now saw this comment. I'll look into these things. Curious if there's a whitelist approach instead of ignore.

* to `out/tmp` as `.cjs` files (renaming of import / export to cjs compatible code).
* 2. requires `dh-core.mjs` and return the default export.
* Copy / modified from https://github.com/deephaven/deephaven.io/blob/main/tools/run-examples/includeAPI.mjs
* NOTE: there is a limitation in current vscode extension apis such that es6 imports are not supported. This is why
Copy link
Member

Choose a reason for hiding this comment

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

Gross

@bmingles bmingles merged commit 0e73aa3 into main Sep 20, 2024
3 checks passed
@bmingles bmingles deleted the 79-1-packages-dir branch September 20, 2024 18:27
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